* Re: Alternates corruption issue
From: Jonathan Nieder @ 2012-01-31 22:59 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Richard Purdie, GIT Mailing-list, Hart, Darren,
Ashfield, Bruce
In-Reply-To: <20120131224240.GA3844@sigill.intra.peff.net>
Jeff King wrote:
> On Tue, Jan 31, 2012 at 04:22:58PM -0600, Jonathan Nieder wrote:
>> I admit part of the reason I care is that just putting "" first would
>> probably taken care of the more important part of
>> <http://bugs.debian.org/399041>.
>
> Would that fix it? If I understand it, the repo in question is bare with
> a ".git" directory inside it.
The layout was foo/.git/.git. The real repository was made with plain
"git init" or "git clone", and then "git svn init" or similar was run
from within .git, creating a .git subdirectory. (Something more must
have happened, actually, since the tree was weirdly sparse:
$ find .git/.git
.git/.git
.git/.git/svn
.git/.git/svn/git-svn
.git/.git/svn/git-svn/.rev_db
)
In the current scheme, to access such a broken repository remotely,
you have to use a URL ".../foo". URLs pointing to foo/.git (like gitk
once used) end up rewritten as .git/.git. Of course, your patch with
the is_git_directory() will fix this specific case. :)
Not a huge deal, especially since "git svn init" prints out where it
is writing these days.
^ permalink raw reply
* Re: [PATCH] pack-objects: convert to use parse_options()
From: Junio C Hamano @ 2012-01-31 23:29 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1328017702-14489-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> @@ -2305,183 +2300,140 @@ static void get_object_list(int ac, const char **av)
> loosen_unused_packed_objects(&revs);
> }
>
> +static int option_parse_index_version(const struct option *opt,
> + const char *arg, int unset)
> +{
> + char *c;
> + const char *val = arg;
> + pack_idx_opts.version = strtoul(val, &c, 10);
> + if (pack_idx_opts.version > 2)
> + die("unsupported index version %s", val);
Dying may have been the appropriate error path in the custom option
parser, but is it still so for a callback in the parse-options framework?
Or should this be a 'return error(_("..."), val)'?
> + if (*c == ',' && c[1])
> + pack_idx_opts.off32_limit = strtoul(c+1, &c, 0);
I do not think the original had this extra "&& c[1]" check and I didn't
see any explanation of this change in the log message, either.
> +static int option_parse_ulong(const struct option *opt,
> + const char *arg, int unset)
> +{
> + if (unset)
> + die("option %s does not accept negative form",
> + opt->long_name);
> +
> + if (!git_parse_ulong(arg, opt->value))
> + die("unable to parse value '%s' for option %s",
> + arg, opt->long_name);
> + return 0;
> +}
Likewise on "die()".
> +#define OPT_ULONG(s, l, v, h) \
> + { OPTION_CALLBACK, (s), (l), (v), "n", (h), \
> + PARSE_OPT_NONEG, option_parse_ulong }
> +
> int cmd_pack_objects(int argc, const char **argv, const char *prefix)
> {
> int use_internal_rev_list = 0;
> int thin = 0;
> int all_progress_implied = 0;
> - uint32_t i;
> - const char **rp_av;
> - int rp_ac_alloc = 64;
> - int rp_ac;
> + const char *rp_av[6];
Nice stackframe shrinkage.
> + int rp_ac = 0;
> + int rev_list_unpacked = 0, rev_list_all = 0, rev_list_reflog = 0;
> + struct option pack_objects_options[] = {
> + OPT_SET_INT('q', "quiet", &progress,
> + "do not show progress meter", 0),
> + OPT_SET_INT(0, "progress", &progress,
> + "show progress meter", 1),
> + OPT_SET_INT(0, "all-progress", &progress,
> + "show progress meter during object writing phase", 2),
Sounds as if the progress is not shown during the counting phase.
Nothing ", too" at the end could not fix, though.
> + OPT_BOOL(0, "all-progress-implied",
> + &all_progress_implied,
> + "similar to --all-progress when progress meter is shown"),
Hrm, interesting wording.
> + { OPTION_CALLBACK, 0, "index-version", NULL, "version",
> + "force generating pack index at a particular version",
> + 0, option_parse_index_version },
Sounds as if you can generate a pack index at HEAD~24, but that is not
what you meant. "version" is too loaded a word that needs qualification.
Perhaps "write the pack index file in the specified idx format version".
"index" is also a loaded word, and our documentation tries to use "idx"
when we talk about the pack index (we say "index" when we mean the
dircache).
> + OPT_INTEGER(0, "window", &window,
> + "limit pack window by objects"),
> + OPT_ULONG(0, "window-memory", &window_memory_limit,
> + "limit pack window by memory"),
> + OPT_INTEGER(0, "depth", &depth,
> + "limit pack window by maximum delta depth"),
These descriptions are somewhat interesting.
The "pack window" is limited by number of objects given by --window (and
by default this is set to 10), but if you give --window-memory, it could
further dynamically shrink it under memory pressure. If the logic were to
use this dynamic shrinkage without any limit on the number of objects in
flight when --window-memory and no --window is given, then you could say
"limit by memory" as if the "pack window" is counted in terms of number of
bytes in the window instead of number of objects and the description would
be accurate. But that is not quite the case as far as I know.
And --depth does not affect the size of the pack window at all. It rejects
an object with deep enough delta chain as a delta-base candidate to limit
the run-time performance penalty for the user of the resulting pack, but
does not stop the caller from trying the next object in the window.
"maximum length of delta chain allowed in the resulting pack", perhaps?
> + OPT_BOOL(0, "reuse-delta", &reuse_delta,
> + "reusing existing deltas"),
> + OPT_BOOL(0, "reuse-object", &reuse_object,
> + "reusing existing objects"),
s/reusing/reuse/???
> + OPT_BOOL(0, "delta-base-offset", &allow_ofs_delta,
> + "use OFS_DELTA objects"),
> + OPT_INTEGER(0, "threads", &delta_search_threads,
> + "use threads when searching for best delta matches"),
> + OPT_BOOL(0, "non-empty", &non_empty,
> + "only create if it would contain at least one object"),
create what? "do not create an empty pack output"?
> + OPT_BOOL(0, "revs", &use_internal_rev_list,
> + "read revision arguments from standard output"),
s/output/input/???
> + { OPTION_SET_INT, 0, "unpacked", &rev_list_unpacked, NULL,
> + "limit the objects to those that are not already packed",
> + PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
s/already/yet/???
> + { OPTION_SET_INT, 0, "all", &rev_list_all, NULL,
> + "include all refs under $GIT_DIR/refs directory",
> + PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
> + { OPTION_SET_INT, 0, "reflog", &rev_list_reflog, NULL,
> + "include objects referred by reflog entries",
> + PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
The latter is more correct---packfile does not pack refs, it packs
objects.
Perhaps former should match: "include objects reachable from any
reference". I would also prefer to avoid "$GIT_DIR/refs _directory_"
to discourage people from running "ls -R .git/refs/".
> + OPT_BOOL(0, "stdout", &pack_to_stdout,
> + "output pack to stdout"),
> + OPT_BOOL(0, "include-tag", &include_tag,
> + "include unasked-for annotated tags"),
"include tag objects that refer to objects to be packed"?
> + OPT_BOOL(0, "keep-unreachable", &keep_unreachable,
> + "keep unreachable objects"),
What does this option mean these days (aka "where are they kept")?
IIRC, this was meant to be used in conjunction with the --unpacked= and
later with --kept-pack-only (see 03a9683d22) and then was later further
modified by 4d6acb70411.
I *think* this meant to include objects that are in packfiles that are not
marked with .keep even when they are not in the revision range to be
packed, so that we won't lose them during gc.
But I think --unpack-unreachable introduced by ca11b21 (let pack-objects
do the writing of unreachable objects as loose objects, 2008-05-14) that
writes these unreachable objects out of the pack to loose form made this
option unnecessary, and it remains unused ever since. It may not be a bad
idea to deprecate this option.
Obviously it is outside of the scope of this patch.
> + OPT_BOOL(0, "unpack-unreachable", &unpack_unreachable,
> + "unpack unreachable objects"),
> + OPT_BOOL(0, "thin", &thin,
> + "create thin packs"),
> + OPT_BOOL(0, "honor-pack-keep", &ignore_packed_keep,
> + "ignore packs that have companion .keep file"),
> + OPT_INTEGER(0, "compression", &pack_compression_level,
> + "pack compression level"),
> + OPT_SET_INT(0, "keep-true-parents", &grafts_replace_parents,
> + "do not hide commits by grafts", 0),
It is noteworthy that this is truly "do not hide".
A graft entry can also add extra commits, but this option will not prevent
the command from including them, so that the result of repacking will keep
the union of parents in the real history (i.e. in commit objects) and
parents in the pretend history (i.e. added by grafts).
> + OPT_END(),
> + };
>
> read_replace_refs = 0;
> ...
> - continue;
> - }
> - usage(pack_usage);
> + if (rev_list_unpacked) {
> + use_internal_rev_list = 1;
> + rp_av[rp_ac++] = "--unpacked";
> + }
> + if (rev_list_all) {
> + use_internal_rev_list = 1;
> + rp_av[rp_ac++] = "--all";
> + }
> + if (rev_list_reflog) {
> + use_internal_rev_list = 1;
> + rp_av[rp_ac++] = "--reflog";
> }
I'd prefer to have "unpacked" the last, to match the order of parameters
in which the typical caller, i.e. repack, calls us, but that is minor.
> @@ -2497,12 +2449,25 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
> * walker.
> */
>
> - if (!pack_to_stdout)
> - base_name = argv[i++];
> -
> - if (pack_to_stdout != !base_name)
> - usage(pack_usage);
> + if (!pack_to_stdout) {
> + if (!argc)
> + die("base name required if --stdout is not given");
> + base_name = argv[0];
> + argc--;
> + }
> + if (argc)
> + die("base name or --stdout are mutually exclusive");
Doesn't this change regress the most basic, i.e.
$ git pack-objects
usage: ...
Other than that, thanks for a pleasant and thought-provoking read.
^ permalink raw reply
* logging disjoint sets of commits in a single command
From: Bryan O'Sullivan @ 2012-02-01 0:15 UTC (permalink / raw)
To: git@vger.kernel.org
I'm trying to use "git log" to display only a handful of commits, where
the commits are not necessarily linearly related to each other.
^ permalink raw reply
* Re: logging disjoint sets of commits in a single command
From: Bryan O'Sullivan @ 2012-02-01 0:27 UTC (permalink / raw)
To: git@vger.kernel.org
In-Reply-To: <CB4DC432.72D%bryano@fb.com>
On 2012-01-31 16:15 , "Bryan O'Sullivan" <bryano@fb.com> wrote:
>I'm trying to use "git log" to display only a handful of commits, where
>the commits are not necessarily linearly related to each other.
And I beautifully fat-fingered the "send" key. Oops.
What I was *going* to say was that it looks like revision.c:limit_list is
(whether intentionally or not) getting in the way of this.
Here's a sample command line against a kernel tree:
git log 373af0c^..373af0c 590dfe2^..590dfe2
I want git to log those two specific commits, but in fact it looks like
limit_list is marking 590dfe2 as UNINTERESTING while processing 373af0c,
and so it gets pruned.
Is there some way around this, or would a patch to fix it be acceptable?
^ permalink raw reply
* Re: [PATCH/RFC v3] grep: Add the option '--exclude'
From: Junio C Hamano @ 2012-02-01 0:31 UTC (permalink / raw)
To: Albert Yale; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <1328021108-4662-1-git-send-email-surfingalbert@gmail.com>
Albert Yale <surfingalbert@gmail.com> writes:
> @@ -124,6 +125,12 @@ OPTIONS
> Use fixed strings for patterns (don't interpret pattern
> as a regex).
>
> +-x <pattern>::
> +--exclude <pattern>::
> + In addition to those found in .gitignore (per directory) and
> + $GIT_DIR/info/exclude, also consider these patterns to be in the
> + set of the ignore rules in effect.
That makes it sound as if "git grep" will not produce hits for a path that
was forcibly added despite it matches a pattern in .gitignore file, which
is not true at all.
> -n::
> --line-number::
> Prefix the line number to matching lines.
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 9ce064a..e9e1ac1 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -528,7 +528,8 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int
> struct cache_entry *ce = active_cache[nr];
> if (!S_ISREG(ce->ce_mode))
> continue;
> - if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL))
> + if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce),
> + 0, NULL, 1))
> continue;
> /*
> * If CE_VALID is on, we assume worktree file and its cache entry
> @@ -566,6 +567,11 @@ 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->exclude,
> + entry.path, strlen(entry.path),
> + 0, NULL, 0))
> + continue;
> +
Why???
IOW, why isn't this
if (!match_pathspec_depth(pathspec, ...))
continue;
My point of the two previous review messages was that it would be nice if
we can make it so that nobody outside match_pathspec_depth() should even
need to know existence of pathspec->exclude. Either that was ignored, or
perhaps you found a compelling reason why that is not a good idea, but if
so I'd like to know why.
I suspect that you do not need to add the extra 0 at the end (it makes the
caller even harder to read) if you go that route. The extra parameter
defeats the whole point of encapsulating the new "negative match" feature
behind match_pathspec_depth() implementation.
> + if (obj->type == OBJ_BLOB) {
> + const char *name_without_sha1 = strchr(name, ':') + 1;
> +
> + if (match_pathspec_depth(pathspec->exclude,
> + name_without_sha1,
> + strlen(name_without_sha1),
> + 0, NULL, 0))
> + return 0;
Likewise.
> diff --git a/cache.h b/cache.h
> index 9bd8c2d..683458a 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -533,9 +533,12 @@ struct pathspec {
> int len;
> unsigned int use_wildcard:1;
> } *items;
> +
> + struct pathspec *exclude; /* This is never NULL. */
> };
"This is never NULL" is blatantly wrong, as pathspec->exclude->exclude
will very likely be NULL.
I've been assuming that the resulting structure would be more like this:
struct pathspec_set {
int nr;
unsigned int has_wildcard:1;
unsigned int recursive:1;
struct pathspec_item {
const char *match;
int len;
unsigned int use_wildcard:1;
} *items;
};
struct pathspec {
const char **raw; /* get_pathspec() result, not freed by free_pathspec() */
int max_depth;
struct pathspec_set include;
struct pathspec_set exclude;
};
I am not including "raw" in the include/exclude because its use should be
deprecated away over time.
The various helpers used in match_pathspec_depth() that know _only_ about
matching and not about excluding will be changed to take a pointer to
"struct pathspec_set" (and possibly max_depth). The top-level callers
would not have to know any of these changes if we structure the API that
way, no?
I think something like that was more or less the structure we discussed
when Nguyễn brought up his negative pathspec work the last time.
^ permalink raw reply
* Re: logging disjoint sets of commits in a single command
From: Carlos Martín Nieto @ 2012-02-01 0:39 UTC (permalink / raw)
To: Bryan O'Sullivan; +Cc: git@vger.kernel.org
In-Reply-To: <CB4DC442.72F%bryano@fb.com>
[-- Attachment #1: Type: text/plain, Size: 1221 bytes --]
On Wed, 2012-02-01 at 00:27 +0000, Bryan O'Sullivan wrote:
> On 2012-01-31 16:15 , "Bryan O'Sullivan" <bryano@fb.com> wrote:
>
> >I'm trying to use "git log" to display only a handful of commits, where
> >the commits are not necessarily linearly related to each other.
>
> And I beautifully fat-fingered the "send" key. Oops.
>
> What I was *going* to say was that it looks like revision.c:limit_list is
> (whether intentionally or not) getting in the way of this.
>
> Here's a sample command line against a kernel tree:
>
> git log 373af0c^..373af0c 590dfe2^..590dfe2
>
> I want git to log those two specific commits, but in fact it looks like
> limit_list is marking 590dfe2 as UNINTERESTING while processing 373af0c,
> and so it gets pruned.
>
> Is there some way around this, or would a patch to fix it be acceptable?
From my reading of the manpage (and the way most git commands work) log
accepts one range of commits. They all get bunched up together.
You might find cat-file's --batch mode interesting.
git rev-list 373af0c^..373af0c | git cat-file --batch
git rev-list 590dfe2^..590dfe2 | git cat-file --batch
looks a lot like what you're looking for.
cmn
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: logging disjoint sets of commits in a single command
From: Junio C Hamano @ 2012-02-01 0:48 UTC (permalink / raw)
To: Bryan O'Sullivan; +Cc: git@vger.kernel.org
In-Reply-To: <CB4DC442.72F%bryano@fb.com>
"Bryan O'Sullivan" <bryano@fb.com> writes:
> Here's a sample command line against a kernel tree:
>
> git log 373af0c^..373af0c 590dfe2^..590dfe2
This command line is _defined_ to be the same as this.
git log ^373af0c^ 373af0c ^590dfe2^ 590dfe2
Hence,
> Is there some way around this, or would a patch to fix it be acceptable?
the answer to the second question is "no, that is not a fix but is a
breakage for the *current* Git users".
The answer to the first question is that you may be able to do something
like this:
(
git rev-list 373af0c^..373af0c
git rev-list 590dfe2^..590dfe2
) |
sort -u |
xargs git show
Having said all that, for users of Git 2.0, giving richer meaning to the
explicit range notation to make your original command line work just like
the above scripted way would be more intuitive. While an unconditional
change to break the current users would totally be unacceptable, we would
want to see somebody come up with a clean migration path toward that goal
without hurting existing users in the longer term.
^ permalink raw reply
* Re: logging disjoint sets of commits in a single command
From: Jeff King @ 2012-02-01 0:53 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: Bryan O'Sullivan, git@vger.kernel.org
In-Reply-To: <1328056769.31804.217.camel@centaur.lab.cmartin.tk>
On Wed, Feb 01, 2012 at 01:39:29AM +0100, Carlos Martín Nieto wrote:
> > Here's a sample command line against a kernel tree:
> >
> > git log 373af0c^..373af0c 590dfe2^..590dfe2
> >
> > I want git to log those two specific commits, but in fact it looks like
> > limit_list is marking 590dfe2 as UNINTERESTING while processing 373af0c,
> > and so it gets pruned.
> >
> > Is there some way around this, or would a patch to fix it be acceptable?
>
> From my reading of the manpage (and the way most git commands work) log
> accepts one range of commits. They all get bunched up together.
Right. That command is equivalent to:
373af0c 590dfe2 --not 373af0c^ 590dfe2^
So the limiting for one range you're interested in ends up marking part
of the other as uninteresting, and that's by design. This topic came up
recently, and I think the general consensus is that it would be cool to
be able to do totally independent ranges, but that would be backwards
incompatible with the current behavior.
In the general case, you can emulate this with:
{ git log 373af0c^..373af0c
git log 590dfe2^..590dfe2
} | $PAGER
which is of course slightly more annoying to type. If you're just
interested in _single_ commits, though, you can just give the commits
and turn off walking:
git log --no-walk 373af0c 590dfe2
> You might find cat-file's --batch mode interesting.
>
> git rev-list 373af0c^..373af0c | git cat-file --batch
> git rev-list 590dfe2^..590dfe2 | git cat-file --batch
>
> looks a lot like what you're looking for.
I think you could even drop the rev-lists in this case, since he just
wants a single commit. However, cat-file lacks the niceties of "log",
like fancy --pretty formatting and automatic diffing against parents.
-Peff
^ permalink raw reply
* Re: logging disjoint sets of commits in a single command
From: Bryan O'Sullivan @ 2012-02-01 1:02 UTC (permalink / raw)
To: Jeff King, Junio C Hamano; +Cc: git@vger.kernel.org
In-Reply-To: <20120201005332.GC30969@sigill.intra.peff.net>
On 2012-01-31, "Jeff King" <peff@peff.net> wrote:
>This topic came up
>recently, and I think the general consensus is that it would be cool to
>be able to do totally independent ranges, but that would be backwards
>incompatible with the current behavior.
That's totally sensible. I hadn't been able to tell from inspection
whether the behaviour was deliberate or not.
>which is of course slightly more annoying to type. If you're just
>interested in _single_ commits, though, you can just give the commits
>and turn off walking:
>
> git log --no-walk 373af0c 590dfe2
Oh, nice! I hadn't seen that option.
By the way, the reason I'm even interested in this in the first place is
that the performance of commands like "git blame" and "git log" on files
and subtrees has become a problem for us (> 10 seconds per invocation,
forecast to get much worse), and I wanted to see whether I could feed "git
log" a specific list of revisions, and if so, whether that could yield
good performance.
I have it in mind to build a secondary index (maintained externally) so
that I can supply these git commands with precise lists of revisions for
much faster response times.
Thanks, guys!
^ permalink raw reply
* Re: [PATCH] Correct singular form in diff summary line for human interaction
From: Nguyen Thai Ngoc Duy @ 2012-02-01 1:32 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, git, Ævar Arnfjörð,
Frederik Schwarzer, Brandon Casey, dickey
In-Reply-To: <7vvcnr92y0.fsf@alter.siamese.dyndns.org>
On Wed, Feb 1, 2012 at 12:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> If there is an environment variable to say "I don't want to see
>> variations on strings intended for humans", can it be spelled as
>> LC_ALL=C?
>
> ...
>
> If we were to touch this, I would prefer to do so unconditionally without
> "hrm, can we reliably guess this is meant for humans?" and release it
> unceremoniously, perhaps as part of the next release that will have a much
> bigger user-visible UI correction to 'merge'.
Unconditionally change is fine to me. There's another implication
that's not mentioned in the commit message, this change also allows
non-English translations. Any objections on i18n or we just keep this
line English only? Personally if scripts do not matter any more, I see
no reasons this line cannot be translated.
--
Duy
^ permalink raw reply
* Re: [PATCH] Correct singular form in diff summary line for human interaction
From: Thomas Dickey @ 2012-02-01 1:45 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Junio C Hamano, Jonathan Nieder, git,
Ævar Arnfjörð, Frederik Schwarzer, Brandon Casey,
dickey
In-Reply-To: <CACsJy8Dd4_Pnvzxww38EfZt8NgRow+qxCReohc45XoNpfJCbYg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 530 bytes --]
On Wed, Feb 01, 2012 at 08:32:43AM +0700, Nguyen Thai Ngoc Duy wrote:
> On Wed, Feb 1, 2012 at 12:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
> >> If there is an environment variable to say "I don't want to see
> >> variations on strings intended for humans", can it be spelled as
> >> LC_ALL=C?
I assume from Neider on the list that this is related to mawk, but I don't
have the email preceding this...
--
Thomas E. Dickey <dickey@invisible-island.net>
http://invisible-island.net
ftp://invisible-island.net
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] Correct singular form in diff summary line for human interaction
From: Thomas Dickey @ 2012-02-01 1:56 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Junio C Hamano, Jonathan Nieder, git,
Ævar Arnfjörð, Frederik Schwarzer, Brandon Casey,
dickey
In-Reply-To: <CACsJy8Dd4_Pnvzxww38EfZt8NgRow+qxCReohc45XoNpfJCbYg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1313 bytes --]
On Wed, Feb 01, 2012 at 08:32:43AM +0700, Nguyen Thai Ngoc Duy wrote:
> On Wed, Feb 1, 2012 at 12:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
> >> If there is an environment variable to say "I don't want to see
> >> variations on strings intended for humans", can it be spelled as
> >> LC_ALL=C?
> >
> > ...
... diffstat (google helped find context)
> > If we were to touch this, I would prefer to do so unconditionally without
> > "hrm, can we reliably guess this is meant for humans?" and release it
> > unceremoniously, perhaps as part of the next release that will have a much
> > bigger user-visible UI correction to 'merge'.
>
> Unconditionally change is fine to me. There's another implication
> that's not mentioned in the commit message, this change also allows
> non-English translations. Any objections on i18n or we just keep this
> line English only? Personally if scripts do not matter any more, I see
> no reasons this line cannot be translated.
I seem to recall that gettext does support plurals...
However, going that route means that even innocuous things like the
parentheses "(+)" can be mangled by translators (guaranteed to break
scripts ;-)
--
Thomas E. Dickey <dickey@invisible-island.net>
http://invisible-island.net
ftp://invisible-island.net
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] Correct singular form in diff summary line for human interaction
From: Nguyen Thai Ngoc Duy @ 2012-02-01 2:37 UTC (permalink / raw)
To: dickey
Cc: Junio C Hamano, Jonathan Nieder, git,
Ævar Arnfjörð, Frederik Schwarzer, Brandon Casey,
dickey
In-Reply-To: <20120201015606.GA24482@debian50-32.invisible-island.net>
On Wed, Feb 1, 2012 at 8:56 AM, Thomas Dickey <dickey@his.com> wrote:
>> > If we were to touch this, I would prefer to do so unconditionally without
>> > "hrm, can we reliably guess this is meant for humans?" and release it
>> > unceremoniously, perhaps as part of the next release that will have a much
>> > bigger user-visible UI correction to 'merge'.
>>
>> Unconditionally change is fine to me. There's another implication
>> that's not mentioned in the commit message, this change also allows
>> non-English translations. Any objections on i18n or we just keep this
>> line English only? Personally if scripts do not matter any more, I see
>> no reasons this line cannot be translated.
>
> I seem to recall that gettext does support plurals...
>
> However, going that route means that even innocuous things like the
> parentheses "(+)" can be mangled by translators (guaranteed to break
> scripts ;-)
We disregard scripts at this point already, I think. "+" and "-"
should not be translated. We could either leave them out of
translatable text, or put a note to translators saying not to
translate them.
--
Duy
^ permalink raw reply
* [BUG] gitk and "Ignore space change" option
From: Oleg Kostyuk @ 2012-02-01 2:42 UTC (permalink / raw)
To: git
Debian/testing, git version 1.7.8.3
There is no possibility to:
1) save value of "Ignore space change" option into config file (~/.gitk)
2) pass some option in cmd line (like --ignore-space-change or -w)
So, there is no other control to "Ignore space change", except via
configuration dialog. This is very inconvenient, and I think this
should be considered as bug.
As solution, any of (1) or (2) could be implemented, but if both -
then this will be fantastic :)
Thanks!
PS: could be useful -
http://stackoverflow.com/questions/8221877/gitk-setting-ignore-space-change-option-to-be-true-by-default
--
Sincerely yours,
Oleg Kostyuk (CUB-UANIC)
^ permalink raw reply
* Re: logging disjoint sets of commits in a single command
From: Jeff King @ 2012-02-01 3:03 UTC (permalink / raw)
To: Bryan O'Sullivan; +Cc: Junio C Hamano, git@vger.kernel.org
In-Reply-To: <CB4DCD5C.747%bryano@fb.com>
On Wed, Feb 01, 2012 at 01:02:57AM +0000, Bryan O'Sullivan wrote:
> By the way, the reason I'm even interested in this in the first place is
> that the performance of commands like "git blame" and "git log" on files
> and subtrees has become a problem for us (> 10 seconds per invocation,
> forecast to get much worse), and I wanted to see whether I could feed "git
> log" a specific list of revisions, and if so, whether that could yield
> good performance.
That sounds kind of slow. Is your repository really gigantic? Have you packed
everything? I'm just curious if there's some other way to make things
faster. Is the repository publicly available?
-Peff
^ permalink raw reply
* Re: [PATCH] Correct singular form in diff summary line for human interaction
From: Junio C Hamano @ 2012-02-01 3:04 UTC (permalink / raw)
To: dickey
Cc: Nguyen Thai Ngoc Duy, Jonathan Nieder, git,
Ævar Arnfjörð, Frederik Schwarzer, Brandon Casey,
dickey
In-Reply-To: <20120201015606.GA24482@debian50-32.invisible-island.net>
Thomas Dickey <dickey@his.com> writes:
> On Wed, Feb 01, 2012 at 08:32:43AM +0700, Nguyen Thai Ngoc Duy wrote:
>> On Wed, Feb 1, 2012 at 12:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> >> If there is an environment variable to say "I don't want to see
>> >> variations on strings intended for humans", can it be spelled as
>> >> LC_ALL=C?
>> >
>> > ...
>
> ... diffstat (google helped find context)
When we show diffstat from "git diff --stat" (or "git apply --stat"), we
currently do not do any singular/plural on the last line of the output
that summarizes the graph, ending up with:
1 files changed, 1 insertions(+), 0 deletions(-)
when there is a single line insertion to a file and nothing else.
My recollection is that our behaviour originally came from our desire to
be as close as what "diffstat" produces, but that does not seem to be the
case. I observed that the output from recent versions of "diffstat" is
much more human friendly. We get these, depending on the input, from
"diffstat" version 1.53:
1 file changed, 1 insertion(+)
1 file changed, 1 deletion(-)
0 files changed
2 files changed, 3 insertions(+), 1 deletion(-)
Namely, it does singular/plural correctly, and omits unnecessary "0
deletions(-)" and "0 insertions(+)".
I was wondering if you remember what the behaviour of older versions of
"diffstat" was, and if it was changed to be more human friendly over
time. It is very possible that I am misremembering this and "diffstat" has
always done the singular/plural correctly and omitted useless "0 lines".
Somehow it seems hard to get hold of older versions of "diffstat", and I
was hoping that I could get that information straight out of the current
maintainer.
Thanks for responding and sorry for the lack of context of the original
message.
^ permalink raw reply
* [PATCH] request-pull: explicitly ask tags/$name to be pulled
From: Junio C Hamano @ 2012-02-01 5:50 UTC (permalink / raw)
To: git; +Cc: Mark Brown
When asking for a tag to be pulled, disambiguate by leaving tags/ prefix
in front of the name of the tag. E.g.
... in the git repository at:
git://example.com/git/git.git/ tags/v1.2.3
for you to fetch changes up to 123456...
This way, older versions of "git pull" can be used to respond to such a
request more easily, as "git pull $URL v1.2.3" did not DWIM to fetch
v1.2.3 tag in older versions. Also this makes it clearer for humans that
the pull request is made for a tag and he should anticipate a signed one.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* http://thread.gmane.org/gmane.linux.kernel/1245709/focus=1245909
triggered this
.../howto/using-signed-tag-in-pull-request.txt | 4 ++--
git-request-pull.sh | 2 +-
t/t5150-request-pull.sh | 6 +-----
3 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/Documentation/howto/using-signed-tag-in-pull-request.txt b/Documentation/howto/using-signed-tag-in-pull-request.txt
index a1351c5..98c0033 100644
--- a/Documentation/howto/using-signed-tag-in-pull-request.txt
+++ b/Documentation/howto/using-signed-tag-in-pull-request.txt
@@ -109,7 +109,7 @@ The resulting msg.txt file begins like so:
are available in the git repository at:
- example.com:/git/froboz.git frotz-for-xyzzy
+ example.com:/git/froboz.git tags/frotz-for-xyzzy
for you to fetch changes up to 703f05ad5835c...:
@@ -141,7 +141,7 @@ After receiving such a pull request message, the integrator fetches and
integrates the tag named in the request, with:
------------
- $ git pull example.com:/git/froboz.git/ frotz-for-xyzzy
+ $ git pull example.com:/git/froboz.git/ tags/frotz-for-xyzzy
------------
This operation will always open an editor to allow the integrator to fine
diff --git a/git-request-pull.sh b/git-request-pull.sh
index 64960d6..e6438e2 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -63,7 +63,7 @@ die "fatal: No commits in common between $base and $head"
find_matching_ref='
sub abbr {
my $ref = shift;
- if ($ref =~ s|refs/heads/|| || $ref =~ s|refs/tags/||) {
+ if ($ref =~ s|^refs/heads/|| || $ref =~ s|^refs/tags/|tags/|) {
return $ref;
} else {
return $ref;
diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index da25bc2..7c1dc64 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -179,11 +179,7 @@ test_expect_success 'request names an appropriate branch' '
read repository &&
read branch
} <digest &&
- {
- test "$branch" = full ||
- test "$branch" = master ||
- test "$branch" = for-upstream
- }
+ test "$branch" = tags/full
'
--
1.7.9.155.gf6ee6
^ permalink raw reply related
* What's cooking in git.git (Jan 2012, #08; Tue, 31)
From: Junio C Hamano @ 2012-02-01 7:19 UTC (permalink / raw)
To: git
What's cooking in git.git (Jan 2012, #08; Tue, 31)
--------------------------------------------------
Here are the topics that have been cooking. Commits prefixed with '-' are
only in 'pu' (proposed updates) while commits prefixed with '+' are in
'next'.
As promised in the recent couple of issues of "What's cooking", the first
batch of topics that have been cooking in 'next' are now in 'master'. The
tip of 'next' hasn't been rewound yet, but it soon will after the second
batch of topics graduate.
Here are the repositories that have my integration branches:
With maint, master, next, pu, todo:
git://git.kernel.org/pub/scm/git/git.git
git://repo.or.cz/alt-git.git
https://code.google.com/p/git-core/
https://github.com/git/git
With only maint and master:
git://git.sourceforge.jp/gitroot/git-core/git.git
git://git-core.git.sourceforge.net/gitroot/git-core/git-core
With all the topics and integration branches:
https://github.com/gitster/git
The preformatted documentation in HTML and man format are found in:
git://git.kernel.org/pub/scm/git/git-{htmldocs,manpages}.git/
git://repo.or.cz/git-{htmldocs,manpages}.git/
https://code.google.com/p/git-{htmldocs,manpages}.git/
https://github.com/gitster/git-{htmldocs,manpages}.git/
--------------------------------------------------
[New Topics]
* fc/zsh-completion (2012-01-30) 4 commits
- (squash to previous?) completion: remove unused code
- completion: simplify __git_remotes
- (squash) completion-style
- completion: be nicer with zsh
Somehow only 2 out of 4-part series seem to have reached the list, missing
the other 2.
* jc/maint-request-pull-for-tag (2012-01-31) 1 commit
- request-pull: explicitly ask tags/$name to be pulled
Usability improvement.
Will merge to 'next'.
* nd/find-pack-entry-recent-cache-invalidation (2012-01-31) 1 commit
- find_pack_entry(): do not keep packed_git pointer locally
Review comments sent.
* nd/pack-objects-parseopt (2012-01-31) 1 commit
- pack-objects: convert to use parse_options()
Review comments sent.
* tr/merge-edit-guidance (2012-01-31) 1 commit
(merged to 'next' on 2012-01-31 at bb678f7)
+ merge: add instructions to the commit message when editing
Will merge to 'master' in the second batch.
--------------------------------------------------
[Graduated to "master"]
* ar/i18n-no-gettext (2012-01-27) 4 commits
(merged to 'next' on 2012-01-27 at 0ecf258)
+ i18n: Do not force USE_GETTEXT_SCHEME=fallthrough on NO_GETTEXT
(merged to 'next' on 2012-01-23 at 694a94e)
+ i18n: Make NO_GETTEXT imply fallthrough scheme in shell l10n
+ add a Makefile switch to avoid gettext translation in shell scripts
+ git-sh-i18n: restructure the logic to compute gettext.sh scheme
* da/maint-mergetool-twoway (2012-01-23) 1 commit
(merged to 'next' on 2012-01-23 at f927323)
+ mergetool: Provide an empty file when needed
Caters to GUI merge backends that cannot merge two files without
a base by giving them an empty file as a "pretend" common ancestor.
* jc/advise-i18n (2011-12-22) 1 commit
(merged to 'next' on 2012-01-23 at 6447013)
+ i18n of multi-line advice messages
Allow localization of advice messages that tend to be longer and
multi-line formatted. For now this is deliberately limited to advise()
interface and not vreportf() in general as touching the latter has
interactions with error() that has plumbing callers whose prefix "error: "
should never be translated.
* jl/submodule-re-add (2012-01-24) 1 commit
(merged to 'next' on 2012-01-26 at 482553e)
+ submodule add: fix breakage when re-adding a deep submodule
"git submodule add" forgot to recompute the name to be stored in .gitmodules
when the module was once added to the superproject and already initialized.
* ks/sort-wildcard-in-makefile (2012-01-22) 1 commit
(merged to 'next' on 2012-01-23 at e2e0c1d)
+ t/Makefile: Use $(sort ...) explicitly where needed
t/Makefile is adjusted to prevent newer versions of GNU make from running
tests in seemingly random order.
* ld/git-p4-branches-and-labels (2012-01-20) 5 commits
(merged to 'next' on 2012-01-23 at 9020ec4)
+ git-p4: label import fails with multiple labels at the same changelist
+ git-p4: add test for p4 labels
+ git-p4: importing labels should cope with missing owner
+ git-p4: cope with labels with empty descriptions
+ git-p4: handle p4 branches and labels containing shell chars
(this branch is used by va/git-p4-branch.)
* nd/clone-detached (2012-01-24) 12 commits
(merged to 'next' on 2012-01-26 at 7b0cc8a)
+ clone: fix up delay cloning conditions
(merged to 'next' on 2012-01-23 at bee31c6)
+ push: do not let configured foreign-vcs permanently clobbered
(merged to 'next' on 2012-01-23 at 9cab64e)
+ clone: print advice on checking out detached HEAD
+ clone: allow --branch to take a tag
+ clone: refuse to clone if --branch points to bogus ref
+ clone: --branch=<branch> always means refs/heads/<branch>
+ clone: delay cloning until after remote HEAD checking
+ clone: factor out remote ref writing
+ clone: factor out HEAD update code
+ clone: factor out checkout code
+ clone: write detached HEAD in bare repositories
+ t5601: add missing && cascade
"git clone" learned to detach the HEAD in the resulting repository when
the source repository's HEAD does not point to a branch.
* rr/sequencer (2012-01-11) 2 commits
(merged to 'next' on 2012-01-23 at f349b56)
+ sequencer: factor code out of revert builtin
+ revert: prepare to move replay_action to header
Moving large chunk of code out of cherry-pick/revert for later reuse,
primarily to prepare for the next cycle.
* tr/grep-l-with-decoration (2012-01-23) 1 commit
(merged to 'next' on 2012-01-23 at 42b8795)
+ grep: fix -l/-L interaction with decoration lines
Using "git grep -l/-L" together with options -W or --break may not make
much sense as the output is to only count the number of hits and there is
no place for file breaks, but the latter options made "-l/-L" to miscount
the hits.
* va/git-p4-branch (2012-01-26) 4 commits
(merged to 'next' on 2012-01-26 at e67c52a)
+ t9801: do not overuse test_must_fail
+ git-p4: Change p4 command invocation
+ git-p4: Add test case for complex branch import
+ git-p4: Search for parent commit on branch creation
(this branch uses ld/git-p4-branches-and-labels.)
--------------------------------------------------
[Stalled]
* jc/advise-push-default (2011-12-18) 1 commit
- push: hint to use push.default=upstream when appropriate
Peff had a good suggestion outlining an updated code structure so that
somebody new can try to dip his or her toes in the development. Any
takers?
Waiting for a reroll.
* ss/git-svn-prompt-sans-terminal (2012-01-04) 3 commits
- fixup! 15eaaf4
- git-svn, perl/Git.pm: extend Git::prompt helper for querying users
(merged to 'next' on 2012-01-05 at 954f125)
+ perl/Git.pm: "prompt" helper to honor GIT_ASKPASS and SSH_ASKPASS
The bottom one has been replaced with a rewrite based on comments from
Ævar. The second one needs more work, both in perl/Git.pm and prompt.c, to
give precedence to tty over SSH_ASKPASS when terminal is available.
Will defer till the next cycle.
* nd/commit-ignore-i-t-a (2012-01-16) 2 commits
- commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees
- cache-tree: update API to take abitrary flags
May want to consider this as fixing an earlier UI mistake, and not as a
feature that devides the userbase.
Will defer till the next cycle.
--------------------------------------------------
[Cooking]
* bl/gitweb-project-filter (2012-01-31) 8 commits
- gitweb: Make project search respect project_filter
- gitweb: improve usability of projects search form
- gitweb: place links to parent directories in page header
- gitweb: show active project_filter in project_list page header
- gitweb: limit links to alternate forms of project_list to active project_filter
- gitweb: add project_filter to limit project list to a subdirectory
- gitweb: prepare git_get_projects_list for use outside 'forks'.
- gitweb: move hard coded .git suffix out of git_get_projects_list
Seems to break test 9502.
* rt/completion-branch-edit-desc (2012-01-29) 1 commit
(merged to 'next' on 2012-01-31 at a0195c8)
+ completion: --edit-description option for git-branch
* jn/svn-fe (2012-01-27) 44 commits
(merged to 'next' on 2012-01-29 at 001a395)
+ vcs-svn/svndiff.c: squelch false "unused" warning from gcc
+ Merge branch 'svn-fe' of git://repo.or.cz/git/jrn into jn/svn-fe
+ vcs-svn: reset first_commit_done in fast_export_init
+ Merge branch 'db/text-delta' into svn-fe
+ vcs-svn: do not initialize report_buffer twice
+ Merge branch 'db/text-delta' into svn-fe
+ vcs-svn: avoid hangs from corrupt deltas
+ vcs-svn: guard against overflow when computing preimage length
+ Merge branch 'db/delta-applier' into db/text-delta
+ vcs-svn: implement text-delta handling
+ Merge branch 'db/delta-applier' into db/text-delta
+ Merge branch 'db/delta-applier' into svn-fe
+ vcs-svn: cap number of bytes read from sliding view
+ test-svn-fe: split off "test-svn-fe -d" into a separate function
+ vcs-svn: let deltas use data from preimage
+ vcs-svn: let deltas use data from postimage
+ vcs-svn: verify that deltas consume all inline data
+ vcs-svn: implement copyfrom_data delta instruction
+ vcs-svn: read instructions from deltas
+ vcs-svn: read inline data from deltas
+ vcs-svn: read the preimage when applying deltas
+ vcs-svn: parse svndiff0 window header
+ vcs-svn: skeleton of an svn delta parser
+ vcs-svn: make buffer_read_binary API more convenient
+ vcs-svn: learn to maintain a sliding view of a file
+ Makefile: list one vcs-svn/xdiff object or header per line
+ Merge branch 'db/svn-fe-code-purge' into svn-fe
+ vcs-svn: drop obj_pool
+ vcs-svn: drop treap
+ vcs-svn: drop string_pool
+ vcs-svn: pass paths through to fast-import
+ Merge branch 'db/strbufs-for-metadata' into db/svn-fe-code-purge
+ Merge branch 'db/length-as-hash' (early part) into db/svn-fe-code-purge
+ Merge branch 'db/vcs-svn-incremental' into svn-fe
+ vcs-svn: avoid using ls command twice
+ vcs-svn: use mark from previous import for parent commit
+ vcs-svn: handle filenames with dq correctly
+ vcs-svn: quote paths correctly for ls command
+ vcs-svn: eliminate repo_tree structure
+ vcs-svn: add a comment before each commit
+ vcs-svn: save marks for imported commits
+ vcs-svn: use higher mark numbers for blobs
+ vcs-svn: set up channel to read fast-import cat-blob response
+ Merge commit 'v1.7.5' into svn-fe
"vcs-svn"/"svn-fe" learned to read dumps with svn-deltas and support
incremental imports.
Will merge to 'master' in the second batch.
* jc/split-blob (2012-01-24) 6 commits
- chunked-object: streaming checkout
- chunked-object: fallback checkout codepaths
- bulk-checkin: support chunked-object encoding
- bulk-checkin: allow the same data to be multiply hashed
- new representation types in the packstream
- varint-in-pack: refactor varint encoding/decoding
Not ready.
I finished the streaming checkout codepath, but as explained in 127b177
(bulk-checkin: support chunked-object encoding, 2011-11-30), these are
still early steps of a long and painful journey. At least pack-objects and
fsck need to learn the new encoding for the series to be usable locally,
and then index-pack/unpack-objects needs to learn it to be used remotely.
Given that I heard a lot of noise that people want large files, and that I
was asked by somebody at GitTogether'11 privately for an advice on how to
pay developers (not me) to help adding necessary support, I am somewhat
dissapointed that the original patch series that was sent almost two
months ago still remains here without much comments and updates from the
developer community. I even made the interface to the logic that decides
where to split chunks easily replaceable, and I deliberately made the
logic in the original patch extremely stupid to entice others, especially
the "bup" fanboys, to come up with a better logic, thinking that giving
people an easy target to shoot for, they may be encouraged to help
out. The plan is not working :-(.
* jc/pull-signed-tag (2012-01-23) 1 commit
(merged to 'next' on 2012-01-23 at 4257553)
+ merge: use editor by default in interactive sessions
"git merge" in an interactive session learned to spawn the editor by
default to let the user edit the auto-generated merge message, to
encourage people to explain their merges better. Legacy scripts can
export MERGE_AUTOEDIT=no to retain the historical behaviour.
Will merge to 'master' in the second batch and deal with any fallout in
'master'.
--------------------------------------------------
[Discarded]
* mh/ref-api-rest (2011-12-12) 35 commits
. repack_without_ref(): call clear_packed_ref_cache()
. read_packed_refs(): keep track of the directory being worked in
. is_refname_available(): query only possibly-conflicting references
. refs: read loose references lazily
. read_loose_refs(): take a (ref_entry *) as argument
. struct ref_dir: store a reference to the enclosing ref_cache
. sort_ref_dir(): take (ref_entry *) instead of (ref_dir *)
. do_for_each_ref_in_dir*(): take (ref_entry *) instead of (ref_dir *)
. add_entry(): take (ref_entry *) instead of (ref_dir *)
. search_ref_dir(): take (ref_entry *) instead of (ref_dir *)
. find_containing_direntry(): use (ref_entry *) instead of (ref_dir *)
. add_ref(): take (ref_entry *) instead of (ref_dir *)
. read_packed_refs(): take (ref_entry *) instead of (ref_dir *)
. find_ref(): take (ref_entry *) instead of (ref_dir *)
. is_refname_available(): take (ref_entry *) instead of (ref_dir *)
. get_loose_refs(): return (ref_entry *) instead of (ref_dir *)
. get_packed_refs(): return (ref_entry *) instead of (ref_dir *)
. refs: wrap top-level ref_dirs in ref_entries
. get_ref_dir(): keep track of the current ref_dir
. do_for_each_ref(): only iterate over the subtree that was requested
. refs: sort ref_dirs lazily
. sort_ref_dir(): do not sort if already sorted
. refs: store references hierarchically
. refs.c: rename ref_array -> ref_dir
. struct ref_entry: nest the value part in a union
. check_refname_component(): return 0 for zero-length components
. free_ref_entry(): new function
. refs.c: reorder definitions more logically
. is_refname_available(): reimplement using do_for_each_ref_in_array()
. names_conflict(): simplify implementation
. names_conflict(): new function, extracted from is_refname_available()
. repack_without_ref(): reimplement using do_for_each_ref_in_array()
. do_for_each_ref_in_arrays(): new function
. do_for_each_ref_in_array(): new function
. do_for_each_ref(): correctly terminate while processesing extra_refs
Will be re-rolled. Discarded without prejudice.
* mm/zsh-completion-regression-fix (2012-01-17) 1 commit
(merged to 'next' on 2012-01-23 at 7bc2e0a)
+ bash-completion: don't add quoted space for ZSH (fix regression)
Superseded by a better fix already in 'master'.
^ permalink raw reply
* Re: [PATCH] Fix an "variable might be used uninitialized" gcc warning
From: Miles Bader @ 2012-02-01 7:16 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ramsay Jones, Junio C Hamano, GIT Mailing-list
In-Reply-To: <20120131194302.GD12443@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
>> The versions which complain are 3.4.4 and 4.1.2, whereas 4.4.0 compiles
>> the code without complaint. So, gcc *may* be getting more sane, but I wouldn't
>> bet on it! :-P
>>
>> I've had examples of this kind of warning, which relies heavily on the
>> analysis performed primarily for the optimizer, come-and-go in gcc before
>
> Yep, judging from the commit message, Junio found the same warning
> in 4.6.2.
>
>> Having said that, unless you are going to decree that the project only
>> supports gcc (and presumably only some particular versions of gcc), then you
>> may well find similar warnings triggered when using other compilers anyway ...
>
> 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. ;-)
It's hard to see how any compiler could detect that "mode" always
receives a value here .... it would have to realize that "stage" always
becomes 2 before the loop is exited, and that seems to depend on
non-trivial properties of external data structures...
-miles
--
Joy, n. An emotion variously excited, but in its highest degree arising from
the contemplation of grief in another.
^ permalink raw reply
* Re: [PATCH] Don't search files with an unset "grep" attribute
From: Junio C Hamano @ 2012-02-01 8:01 UTC (permalink / raw)
To: Jeff King; +Cc: Conrad Irwin, git, Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <20120125214625.GA4666@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Jan 23, 2012 at 10:59:45PM -0800, Junio C Hamano wrote:
>
>> Conrad Irwin <conrad.irwin@gmail.com> writes:
>> > I used to use this approach, hooking into the "diff" attribute directly to mark
>> > a file as binary, however that was clearly a hack.
>>
>> After thinking about this a bit more, I have to say I disagree that it is
>> a hack.
>
> I kind of agree.
>
> The biggest problem is that the name is wrong. The "diff.*.command"
> option really is about generating a diff between two blobs of a certain
> type. But "diff.*.textconv" and "diff.*.binary" are really just
> attributes of the file, and may or may not have to do with generating a
> diff. Ditto for diff.*.funcname, I think.
>
> You argue, and I agree, that if we are talking about attributes of the
> files and not diff-specific things, then other parts of git can and
> should make use of that information.
>
> So if this was all spelled:
>
> $ cat .gitattributes
> *.pdf filetype=pdf
> $ cat .git/config
> [filetype "pdf"]
> binary = true
> textconv = pdf2txt
>
> I think it would be a no-brainer that those type attributes should apply
> to "git grep".
I think this discussion has, instead of forking into two equally
interesting subthreads, veered to a more intellectually stimulating
tangent and we ended up losing focus.
Regardless of what to do with "I do not want to grep in these types of
files" and "I want textconv applied when grepping in these types", which
would be new attributes to implement two new features, I would like to see
us first concentrate on fixing the "binary" issue. When somebody tells us
"Your autodetection may screw it up, but this file is binary; just show
'Binary files differ.' when comparing." with "-diff" (or "binary"), we
should honor that when "git grep" decides if it should take the 'Binary
file matches' codepath. We currently do not, and it clearly is a bug.
This is especially made somewhat urgent because I do not want a half-baked
"two pathspecs" approach that only "git grep" knows about when we add the
support for "git grep --exclude-path=...".
We should have to teach the underlying machinery that matches pathspec
about negative pathspec entries only once. After we have done so, all the
callers, not just "git grep", should be able to take advantage of the
change by just learning to place negative pathspec entries in the "struct
pathspec" they pass to the machinery. Doing anything else will lead to
madness of adding ad-hoc "here we should further filter with the other
negative 'struct pathspec'" in each and every application.
But I suspect that it would not materialize anytime soon. And I also
suspect that the correct handling of 'Binary file matches', which is a
pure bugfix, should solve the original issue started these threads 90% in
practice.
^ permalink raw reply
* Re: [PATCH] Don't search files with an unset "grep" attribute
From: Jeff King @ 2012-02-01 8:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Conrad Irwin, git, Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <7vhazb3rtm.fsf@alter.siamese.dyndns.org>
On Wed, Feb 01, 2012 at 12:01:41AM -0800, Junio C Hamano wrote:
> > So if this was all spelled:
> >
> > $ cat .gitattributes
> > *.pdf filetype=pdf
> > $ cat .git/config
> > [filetype "pdf"]
> > binary = true
> > textconv = pdf2txt
> >
> > I think it would be a no-brainer that those type attributes should apply
> > to "git grep".
>
> I think this discussion has, instead of forking into two equally
> interesting subthreads, veered to a more intellectually stimulating
> tangent and we ended up losing focus.
That's what I'm here for.
> Regardless of what to do with "I do not want to grep in these types of
> files" and "I want textconv applied when grepping in these types", which
> would be new attributes to implement two new features, I would like to see
> us first concentrate on fixing the "binary" issue. When somebody tells us
> "Your autodetection may screw it up, but this file is binary; just show
> 'Binary files differ.' when comparing." with "-diff" (or "binary"), we
> should honor that when "git grep" decides if it should take the 'Binary
> file matches' codepath. We currently do not, and it clearly is a bug.
Right. It may have been lost in the verbosity of what I wrote in my
previous email, but I completely agree. With the caveat that one should
also respect "diff=foo" coupled with "diff.foo.binary = true" as making
something binary. But that is already handled transparently by the
userdiff.[ch] code (which seems like the obvious entry point for
grep to use for attribute lookup, and which we already use there for
funcname lookup).
The trivial-ish patch is below.
> We should have to teach the underlying machinery that matches pathspec
> about negative pathspec entries only once. After we have done so, all the
> callers, not just "git grep", should be able to take advantage of the
> change by just learning to place negative pathspec entries in the "struct
> pathspec" they pass to the machinery. Doing anything else will lead to
> madness of adding ad-hoc "here we should further filter with the other
> negative 'struct pathspec'" in each and every application.
Yes, I agree.
> But I suspect that it would not materialize anytime soon. And I also
> suspect that the correct handling of 'Binary file matches', which is a
> pure bugfix, should solve the original issue started these threads 90% in
> practice.
Also agree. Let's fix the bug and then give it some time to see whether
people really want more explicit exclusions.
Here's the bug-fix patch. Not quite ready for inclusion, as it obviously
needs tests and a commit message. Also, we can cache the result of the
userdiff lookup so the funcname code doesn't have to look it up again.
---
diff --git a/grep.c b/grep.c
index b29d09c..d7ab054 100644
--- a/grep.c
+++ b/grep.c
@@ -960,6 +960,15 @@ static void std_output(struct grep_opt *opt, const void *buf, size_t size)
fwrite(buf, size, 1, stdout);
}
+static int grep_buffer_is_binary(const char *path,
+ char *buf, unsigned long size)
+{
+ struct userdiff_driver *drv = userdiff_find_by_path(path);
+ if (drv && drv->binary != -1)
+ return drv->binary;
+ return buffer_is_binary(buf, size);
+}
+
static int grep_buffer_1(struct grep_opt *opt, const char *name,
char *buf, unsigned long size, int collect_hits)
{
@@ -994,11 +1003,11 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
switch (opt->binary) {
case GREP_BINARY_DEFAULT:
- if (buffer_is_binary(buf, size))
+ if (grep_buffer_is_binary(name, buf, size))
binary_match_only = 1;
break;
case GREP_BINARY_NOMATCH:
- if (buffer_is_binary(buf, size))
+ if (grep_buffer_is_binary(name, buf, size))
return 0; /* Assume unmatch */
break;
case GREP_BINARY_TEXT:
^ permalink raw reply related
* Re: [PATCH] Don't search files with an unset "grep" attribute
From: Jeff King @ 2012-02-01 9:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Conrad Irwin, git, Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <20120201082005.GA32348@sigill.intra.peff.net>
On Wed, Feb 01, 2012 at 03:20:05AM -0500, Jeff King wrote:
> Here's the bug-fix patch. Not quite ready for inclusion, as it obviously
> needs tests and a commit message. Also, we can cache the result of the
> userdiff lookup so the funcname code doesn't have to look it up again.
Actually, it's a little bit more complicated. I was looking at a
slightly old version of grep.c. Since 0579f91 (grep: enable threading
with -p and -W using lazy attribute lookup, 2011-12-12), the lookup
happens in lots of sub-functions, and locking is required.
So this is what the patch looks like with proper locking and caching of
the looked-up driver. It's quite messy because the cached driver pointer
has to get passed around quite a bit. And I'm not sure it buys that much
in practice. The cost of attribute lookup _is_ noticeable (which I'll
discuss below), but funcname lookup only happens when we get a grep hit.
So unless you are searching for something extremely common, you're only
going to do a lookup very occasionally (compared to the load of actually
searching through the files). So all of the messiness and caching may
not be worth the effort, as I wasn't able to measure a performance gain.
But there's more. Respecting binary attributes does mean looking up
attributes for _every_ file. And that has a noticeable impact. My
best-of-five for "git grep foo" on linux-2.6 went from 0.302s to 0.392s.
Yuck.
Part of the problem, I suspect, is that the attribute lookup code is
optimized for locality. We only unwind as much of the stack as we need,
so looking at "foo/bar/baz.c" after "foo/bar/bleep.c" is much cheaper
than looking at "some/other/directory.c". But with threaded grep, that
locality is likely lost, as we are mixing up attribute requests from
different threads.
Given that binary lookup means we need every file's gitattribute, it
might be better to look them up serially at the beginning of the
program, and then pass the resulting userdiff driver to grep_buffer
along with each path.
---
diff --git a/grep.c b/grep.c
index 486230b..3ca840a 100644
--- a/grep.c
+++ b/grep.c
@@ -829,15 +829,28 @@ static inline void grep_attr_unlock(struct grep_opt *opt)
#define grep_attr_unlock(opt)
#endif
-static int match_funcname(struct grep_opt *opt, const char *name, char *bol, char *eol)
+static struct userdiff_driver *get_cached_userdiff(struct grep_opt *opt,
+ const char *path,
+ struct userdiff_driver **drv)
{
- xdemitconf_t *xecfg = opt->priv;
- if (xecfg && !xecfg->find_func) {
- struct userdiff_driver *drv;
+ if (!*drv) {
grep_attr_lock(opt);
- drv = userdiff_find_by_path(name);
+ *drv = userdiff_find_by_path(path);
+ if (!*drv)
+ *drv = userdiff_find_by_name("default");
grep_attr_unlock(opt);
- if (drv && drv->funcname.pattern) {
+ }
+ return *drv;
+}
+
+static int match_funcname(struct grep_opt *opt, const char *name,
+ struct userdiff_driver **drv_p,
+ char *bol, char *eol)
+{
+ xdemitconf_t *xecfg = opt->priv;
+ if (xecfg && !xecfg->find_func) {
+ struct userdiff_driver *drv = get_cached_userdiff(opt, name, drv_p);
+ if (drv->funcname.pattern) {
const struct userdiff_funcname *pe = &drv->funcname;
xdiff_set_find_func(xecfg, pe->pattern, pe->cflags);
} else {
@@ -859,6 +872,7 @@ static int match_funcname(struct grep_opt *opt, const char *name, char *bol, cha
}
static void show_funcname_line(struct grep_opt *opt, const char *name,
+ struct userdiff_driver **drv_p,
char *buf, char *bol, unsigned lno)
{
while (bol > buf) {
@@ -871,20 +885,21 @@ static void show_funcname_line(struct grep_opt *opt, const char *name,
if (lno <= opt->last_shown)
break;
- if (match_funcname(opt, name, bol, eol)) {
+ if (match_funcname(opt, name, drv_p, bol, eol)) {
show_line(opt, bol, eol, name, lno, '=');
break;
}
}
}
-static void show_pre_context(struct grep_opt *opt, const char *name, char *buf,
- char *bol, char *end, unsigned lno)
+static void show_pre_context(struct grep_opt *opt, const char *name,
+ struct userdiff_driver **drv_p,
+ char *buf, char *bol, char *end, unsigned lno)
{
unsigned cur = lno, from = 1, funcname_lno = 0;
int funcname_needed = !!opt->funcname;
- if (opt->funcbody && !match_funcname(opt, name, bol, end))
+ if (opt->funcbody && !match_funcname(opt, name, drv_p, bol, end))
funcname_needed = 2;
if (opt->pre_context < lno)
@@ -900,7 +915,7 @@ static void show_pre_context(struct grep_opt *opt, const char *name, char *buf,
while (bol > buf && bol[-1] != '\n')
bol--;
cur--;
- if (funcname_needed && match_funcname(opt, name, bol, eol)) {
+ if (funcname_needed && match_funcname(opt, name, drv_p, bol, eol)) {
funcname_lno = cur;
funcname_needed = 0;
}
@@ -908,7 +923,7 @@ static void show_pre_context(struct grep_opt *opt, const char *name, char *buf,
/* We need to look even further back to find a function signature. */
if (opt->funcname && funcname_needed)
- show_funcname_line(opt, name, buf, bol, cur);
+ show_funcname_line(opt, name, drv_p, buf, bol, cur);
/* Back forward. */
while (cur < lno) {
@@ -983,6 +998,17 @@ static void std_output(struct grep_opt *opt, const void *buf, size_t size)
fwrite(buf, size, 1, stdout);
}
+static int grep_buffer_is_binary(struct grep_opt *opt,
+ const char *path,
+ char *buf, unsigned long size,
+ struct userdiff_driver **drv_p)
+{
+ struct userdiff_driver *drv = get_cached_userdiff(opt, path, drv_p);
+ if (drv && drv->binary != -1)
+ return drv->binary;
+ return buffer_is_binary(buf, size);
+}
+
static int grep_buffer_1(struct grep_opt *opt, const char *name,
char *buf, unsigned long size, int collect_hits)
{
@@ -996,6 +1022,7 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
int show_function = 0;
enum grep_context ctx = GREP_CONTEXT_HEAD;
xdemitconf_t xecfg;
+ struct userdiff_driver *drv = NULL;
if (!opt->output)
opt->output = std_output;
@@ -1017,11 +1044,11 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
switch (opt->binary) {
case GREP_BINARY_DEFAULT:
- if (buffer_is_binary(buf, size))
+ if (grep_buffer_is_binary(opt, name, buf, size, &drv))
binary_match_only = 1;
break;
case GREP_BINARY_NOMATCH:
- if (buffer_is_binary(buf, size))
+ if (grep_buffer_is_binary(opt, name, buf, size, &drv))
return 0; /* Assume unmatch */
break;
case GREP_BINARY_TEXT:
@@ -1099,16 +1126,16 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
* pre-context lines, we would need to show them.
*/
if (opt->pre_context || opt->funcbody)
- show_pre_context(opt, name, buf, bol, eol, lno);
+ show_pre_context(opt, name, &drv, buf, bol, eol, lno);
else if (opt->funcname)
- show_funcname_line(opt, name, buf, bol, lno);
+ show_funcname_line(opt, name, &drv, buf, bol, lno);
show_line(opt, bol, eol, name, lno, ':');
last_hit = lno;
if (opt->funcbody)
show_function = 1;
goto next_line;
}
- if (show_function && match_funcname(opt, name, bol, eol))
+ if (show_function && match_funcname(opt, name, &drv, bol, eol))
show_function = 0;
if (show_function ||
(last_hit && lno <= last_hit + opt->post_context)) {
^ permalink raw reply related
* Re: [PATCH] Don't search files with an unset "grep" attribute
From: Conrad Irwin @ 2012-02-01 9:28 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <20120201091009.GA20984@sigill.intra.peff.net>
On Wed, Feb 1, 2012 at 1:10 AM, Jeff King <peff@peff.net> wrote:
> On Wed, Feb 01, 2012 at 03:20:05AM -0500, Jeff King wrote:
>
> Actually, it's a little bit more complicated. I was looking at a
> slightly old version of grep.c. Since 0579f91 (grep: enable threading
> with -p and -W using lazy attribute lookup, 2011-12-12), the lookup
> happens in lots of sub-functions, and locking is required.
Heh, you just beat me to it.
> But there's more. Respecting binary attributes does mean looking up
> attributes for _every_ file. And that has a noticeable impact. My
> best-of-five for "git grep foo" on linux-2.6 went from 0.302s to 0.392s.
> Yuck.
The first time I introduced this behaviour[1], I made it conditional
on a preference — those who wanted "good" grep could set the
preference, while those who wanted "fast" grep could not. I think
that's not a good idea, though if the performance issues are
show-stoppers, I'd suggest the opposite preference (so speed-freaks
can disable the checks).
Tests from [1] included below in case they're still useful (they pass
with your change)
[1] http://article.gmane.org/gmane.comp.version-control.git/179299/match=grep
---
diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh
index 917a264..4d94461 100755
--- a/t/t7008-grep-binary.sh
+++ b/t/t7008-grep-binary.sh
@@ -99,4 +99,23 @@ test_expect_success 'git grep y<NUL>x a' "
test_must_fail git grep -f f a
"
+test_expect_success 'git -c grep.binaryFiles=1 grep ina a' "
+ echo 'a diff' > .gitattributes &&
+ printf 'binaryQfile' | q_to_nul >a &&
+ echo 'a:binaryQfile' | q_to_nul >expect &&
+ git -c grep.binaryFiles=1 grep ina a > actual &&
+ rm .gitattributes &&
+ test_cmp expect actual
+"
+test_expect_success 'git -c grep.binaryFiles=1 grep tex t' "
+ echo 'text' > t &&
+ git add t &&
+ echo 't -diff' > .gitattributes &&
+ echo Binary file t matches >expect &&
+ git -c grep.binaryFiles=1 grep tex t >actual &&
+ rm .gitattributes &&
+ test_cmp expect actual
+"
+
+
test_done
^ permalink raw reply related
* Re: [PATCH] Correct singular form in diff summary line for human interaction
From: Thomas Dickey @ 2012-02-01 9:40 UTC (permalink / raw)
To: Junio C Hamano
Cc: dickey, Nguyen Thai Ngoc Duy, Jonathan Nieder, git,
Ævar Arnfjörð, Frederik Schwarzer, Brandon Casey,
dickey
In-Reply-To: <7vk4475k5s.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 3686 bytes --]
On Tue, Jan 31, 2012 at 07:04:15PM -0800, Junio C Hamano wrote:
> Thomas Dickey <dickey@his.com> writes:
>
> > On Wed, Feb 01, 2012 at 08:32:43AM +0700, Nguyen Thai Ngoc Duy wrote:
> >> On Wed, Feb 1, 2012 at 12:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
> >> >> If there is an environment variable to say "I don't want to see
> >> >> variations on strings intended for humans", can it be spelled as
> >> >> LC_ALL=C?
> >> >
> >> > ...
> >
> > ... diffstat (google helped find context)
>
> When we show diffstat from "git diff --stat" (or "git apply --stat"), we
> currently do not do any singular/plural on the last line of the output
> that summarizes the graph, ending up with:
>
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> when there is a single line insertion to a file and nothing else.
>
> My recollection is that our behaviour originally came from our desire to
> be as close as what "diffstat" produces, but that does not seem to be the
> case. I observed that the output from recent versions of "diffstat" is
> much more human friendly. We get these, depending on the input, from
> "diffstat" version 1.53:
I added the PLURAL() macro in 1.22, in 1996/3/16, which was a few months
before Tony Nugent commented that it was being used by other people. Since
I'd been sending patches with diffstat's since mid-1994, it's possible
that there were a few copies using the form without plurals. But it's
been quite a while. Also, it was in 1998/1/17 that I modified the
copyright notice (1.26) at the request of someone in the Linux group, so
I'd assume that they would have started using the newer version.
Except for later applying PLURAL to the number of files changed (which Jean
Delvare pointed out in 2005), the message construction has not changed since
then. The insert/delete/modify parts of the message were optional as you see in
my earliest version (1.3) from 1993/10/23:
printf("%d files changed", num_files);
if (total_ins) printf(", %d insertions", total_ins);
if (total_del) printf(", %d deletions", total_del);
if (total_mod) printf(", %d modifications", total_mod);
printf("\n");
> 1 file changed, 1 insertion(+)
> 1 file changed, 1 deletion(-)
> 0 files changed
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> Namely, it does singular/plural correctly, and omits unnecessary "0
> deletions(-)" and "0 insertions(+)".
>
> I was wondering if you remember what the behaviour of older versions of
> "diffstat" was, and if it was changed to be more human friendly over
> time. It is very possible that I am misremembering this and "diffstat" has
> always done the singular/plural correctly and omitted useless "0 lines".
I have back to 1.3 in my archive. I use rcshist to look for things like this.
(also, for small things like this, I have a script that pulls all versions,
with proper timestamps to make it simpler to sort the files by date).
> Somehow it seems hard to get hold of older versions of "diffstat", and I
> was hoping that I could get that information straight out of the current
> maintainer.
I can provide the information.
Actually a couple of weeks ago I was experimenting with rcs-fast-export, but
found that would need more work to export diffstat (I use branches a lot)
I'm using "conflict" as a test-case for the changes that I'm making to support
exporting mawk.
> Thanks for responding and sorry for the lack of context of the original
> message.
no problem - google was helpful ;-)
--
Thomas E. Dickey <dickey@invisible-island.net>
http://invisible-island.net
ftp://invisible-island.net
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* ANNOUNCE: Git for Windows 1.7.9
From: Pat Thoyts @ 2012-02-01 11:23 UTC (permalink / raw)
To: msysGit; +Cc: Git Mailing List
This release brings the latest release of Git to Windows users.
Pre-built installers are available from
http://code.google.com/p/msysgit/downloads/list
Further details about the Git for Windows project are at
http://code.google.com/p/msysgit/
^ 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