* [PATCH 2/2] grep: respect diff attributes for binary-ness
From: Jeff King @ 2012-02-01 23:21 UTC (permalink / raw)
To: Conrad Irwin; +Cc: Junio C Hamano, git, Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <20120201221437.GA19044@sigill.intra.peff.net>
There is currently no way for users to tell git-grep that a
particular path is or is not a binary file; instead, grep
always relies on its auto-detection (or the user specifying
"-a" to treat all binary-looking files like text).
This patch teaches git-grep to use the same attribute lookup
that is used by git-diff. We could add a new "grep" flag,
but that is unnecessarily complex and unlikely to be useful.
Despite the name, the "-diff" attribute (or "diff=foo" and
the associated diff.foo.binary config option) are really
about describing the contents of the path. It's simply
historical that diff was the only thing that cared about
these attributes in the past.
And if this simple approach turns out to be insufficient, we
still have a backwards-compatible path forward: we can add a
separate "grep" attribute, and fall back to respecting
"diff" if it is unset.
There are a few things worth nothing about the
implementation.
One is that the attribute lookup happens outside of the
grep.[ch] interface (i.e., outside of grep_buffer). We could
do it at a lower level, which would be slightly more
convenient for callers. However, this interacts badly with
threading. The attribute-lookup code performs best when
lookup order matches the filesystem order (so looking up
"a/b/c" is cheaper if we just looked up "a/b/d" than if we
just did "x/y/z"). Because we issue many simultaneous
requests to grep_buffer, performing the attribute lookup at
that level will cause requests from unrelated paths to
interleave, and we lose the locality that makes the lookup
optimization work.
Instead, in the threaded case we check the attributes as
they are added to the work queue, meaning they are looked up
in the optimal order (in the single threaded case, this is a
non-issue, as we process the files serially in the optimal
order).
Here are a few numbers showing the difference. The first is
a best-of-five time for "git grep foo" on the linux-2.6 repo
before this patch (on a 4-core HT processor, using 8
threads):
real 0m0.306s
user 0m1.512s
sys 0m0.412s
Now here's the time for the same operation with a trial
implementation looking up attributes in grep_buffer,
showing a 31% slowdown:
real 0m0.401s
user 0m1.760s
sys 0m0.636s
And here's the same operation with this patch, with only an
11% slowdown:
real 0m0.339s
user 0m1.444s
sys 0m0.584s
Note that while the percentages are big, the absolute
numbers are pretty small. In particular, this is a very
inexpensive grep to do. A more complex regex should have the
same absolute slowdown from the attribute lookup, but it
would be a much smaller percentage of the total processing
time. So doing it this way is not a huge win, but it does
help on small greps.
The second issue worth noting is that while we do a full
attribute lookup, we pass along only the binary flag to
grep_buffer. When the "-p" flag is given to grep, we will
actually look up the same attributes to find funcname
patterns of matches. We could pass along the pointer to the
userdiff driver for reuse.
However, it's not worth doing this. It clutters the code, as
the driver has to be passed through a large number of helper
functions (and pollutes the grep_buffer interface with
userdiff code). And in my tests, it didn't actually improve
performance. Because we only have to look up the attribute
for a grep hit, in most cases we will only do the funcname
lookup for a small subset of files. The cost of the extra
lookups turns out to be negligible.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/grep.c | 26 ++++++++++++++++++++++----
t/t7008-grep-binary.sh | 24 ++++++++++++++++++++++++
2 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index e328316..bb38804 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -42,6 +42,7 @@ enum work_type {WORK_SHA1, WORK_FILE};
struct work_item {
enum work_type type;
char *name;
+ int is_binary;
/* if type == WORK_SHA1, then 'identifier' is a SHA1,
* otherwise type == WORK_FILE, and 'identifier' is a NUL
@@ -113,6 +114,14 @@ static pthread_cond_t cond_result;
static int skip_first_line;
+static int path_is_binary(const char *path)
+{
+ struct userdiff_driver *drv = userdiff_find_by_path(path);
+ if (drv)
+ return drv->binary;
+ return -1;
+}
+
static void add_work(enum work_type type, char *name, void *id)
{
grep_lock();
@@ -123,6 +132,11 @@ static void add_work(enum work_type type, char *name, void *id)
todo[todo_end].type = type;
todo[todo_end].name = name;
+
+ pthread_mutex_lock(&grep_attr_mutex);
+ todo[todo_end].is_binary = path_is_binary(name);
+ pthread_mutex_unlock(&grep_attr_mutex);
+
todo[todo_end].identifier = id;
todo[todo_end].done = 0;
strbuf_reset(&todo[todo_end].out);
@@ -221,14 +235,16 @@ static void *run(void *arg)
void* data = load_sha1(w->identifier, &sz, w->name);
if (data) {
- hit |= grep_buffer(opt, w->name, -1, data, sz);
+ hit |= grep_buffer(opt, w->name, w->is_binary,
+ data, sz);
free(data);
}
} else if (w->type == WORK_FILE) {
size_t sz;
void* data = load_file(w->identifier, &sz);
if (data) {
- hit |= grep_buffer(opt, w->name, -1, data, sz);
+ hit |= grep_buffer(opt, w->name, w->is_binary,
+ data, sz);
free(data);
}
} else {
@@ -421,7 +437,8 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
if (!data)
hit = 0;
else
- hit = grep_buffer(opt, name, -1, data, sz);
+ hit = grep_buffer(opt, name, path_is_binary(name),
+ data, sz);
free(data);
free(name);
@@ -483,7 +500,8 @@ static int grep_file(struct grep_opt *opt, const char *filename)
if (!data)
hit = 0;
else
- hit = grep_buffer(opt, name, -1, data, sz);
+ hit = grep_buffer(opt, name, path_is_binary(name),
+ data, sz);
free(data);
free(name);
diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh
index 917a264..fd6410f 100755
--- a/t/t7008-grep-binary.sh
+++ b/t/t7008-grep-binary.sh
@@ -99,4 +99,28 @@ test_expect_success 'git grep y<NUL>x a' "
test_must_fail git grep -f f a
"
+test_expect_success 'grep respects binary diff attribute' '
+ echo text >t &&
+ git add t &&
+ echo t:text >expect &&
+ git grep text t >actual &&
+ test_cmp expect actual &&
+ echo "t -diff" >.gitattributes &&
+ echo "Binary file t matches" >expect &&
+ git grep text t >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep respects not-binary diff attribute' '
+ echo binQary | q_to_nul >b &&
+ git add b &&
+ echo "Binary file b matches" >expect &&
+ git grep bin b >actual &&
+ test_cmp expect actual &&
+ echo "b diff" >.gitattributes &&
+ echo "b:binQary" >expect &&
+ git grep bin b | nul_to_q >actual &&
+ test_cmp expect actual
+'
+
test_done
--
1.7.9.3.gc3fce1.dirty
^ permalink raw reply related
* [PATCH 1/2] grep: let grep_buffer callers specify a binary flag
From: Jeff King @ 2012-02-01 23:21 UTC (permalink / raw)
To: Conrad Irwin; +Cc: Junio C Hamano, git, Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <20120201221437.GA19044@sigill.intra.peff.net>
The caller of grep_buffer may have extra information about
whether a buffer is binary or not (e.g., from configuration).
Let's give them a chance to pass along that information and
override our binary auto-detection.
Callers can still pass "-1" to get the regular
auto-detection (and all callers are converted to do this,
meaning there should be no behavior change yet).
We could maintain source compatibility for callers by adding
a new "grep_buffer_with_flags" and leaving "grep_buffer" as
a wrapper that always passes "-1". But there are only 5
callers of grep_buffer, and only 1 of those (grepping commit
buffers) will not be converted to pass something useful in
the next patch. So it's simpler to just add a "-1" there.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/grep.c | 8 ++++----
grep.c | 23 ++++++++++++++++-------
grep.h | 2 +-
revision.c | 1 +
4 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 9ce064a..e328316 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -221,14 +221,14 @@ static void *run(void *arg)
void* data = load_sha1(w->identifier, &sz, w->name);
if (data) {
- hit |= grep_buffer(opt, w->name, data, sz);
+ hit |= grep_buffer(opt, w->name, -1, data, sz);
free(data);
}
} else if (w->type == WORK_FILE) {
size_t sz;
void* data = load_file(w->identifier, &sz);
if (data) {
- hit |= grep_buffer(opt, w->name, data, sz);
+ hit |= grep_buffer(opt, w->name, -1, data, sz);
free(data);
}
} else {
@@ -421,7 +421,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
if (!data)
hit = 0;
else
- hit = grep_buffer(opt, name, data, sz);
+ hit = grep_buffer(opt, name, -1, data, sz);
free(data);
free(name);
@@ -483,7 +483,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)
if (!data)
hit = 0;
else
- hit = grep_buffer(opt, name, data, sz);
+ hit = grep_buffer(opt, name, -1, data, sz);
free(data);
free(name);
diff --git a/grep.c b/grep.c
index 486230b..e547db2 100644
--- a/grep.c
+++ b/grep.c
@@ -983,8 +983,16 @@ 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(char *buf, unsigned long size, int flag)
+{
+ if (flag == -1)
+ flag = buffer_is_binary(buf, size);
+ return flag;
+}
+
static int grep_buffer_1(struct grep_opt *opt, const char *name,
- char *buf, unsigned long size, int collect_hits)
+ int is_binary, char *buf, unsigned long size,
+ int collect_hits)
{
char *bol = buf;
unsigned long left = size;
@@ -1017,11 +1025,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(buf, size, is_binary))
binary_match_only = 1;
break;
case GREP_BINARY_NOMATCH:
- if (buffer_is_binary(buf, size))
+ if (grep_buffer_is_binary(buf, size, is_binary))
return 0; /* Assume unmatch */
break;
case GREP_BINARY_TEXT:
@@ -1182,23 +1190,24 @@ static int chk_hit_marker(struct grep_expr *x)
}
}
-int grep_buffer(struct grep_opt *opt, const char *name, char *buf, unsigned long size)
+int grep_buffer(struct grep_opt *opt, const char *name, int is_binary,
+ char *buf, unsigned long size)
{
/*
* we do not have to do the two-pass grep when we do not check
* buffer-wide "all-match".
*/
if (!opt->all_match)
- return grep_buffer_1(opt, name, buf, size, 0);
+ return grep_buffer_1(opt, name, is_binary, buf, size, 0);
/* Otherwise the toplevel "or" terms hit a bit differently.
* We first clear hit markers from them.
*/
clr_hit_marker(opt->pattern_expression);
- grep_buffer_1(opt, name, buf, size, 1);
+ grep_buffer_1(opt, name, is_binary, buf, size, 1);
if (!chk_hit_marker(opt->pattern_expression))
return 0;
- return grep_buffer_1(opt, name, buf, size, 0);
+ return grep_buffer_1(opt, name, is_binary, buf, size, 0);
}
diff --git a/grep.h b/grep.h
index fb205f3..8447e4c 100644
--- a/grep.h
+++ b/grep.h
@@ -128,7 +128,7 @@ extern void append_grep_pattern(struct grep_opt *opt, const char *pat, const cha
extern void append_header_grep_pattern(struct grep_opt *, enum grep_header_field, const char *);
extern void compile_grep_patterns(struct grep_opt *opt);
extern void free_grep_patterns(struct grep_opt *opt);
-extern int grep_buffer(struct grep_opt *opt, const char *name, char *buf, unsigned long size);
+extern int grep_buffer(struct grep_opt *opt, const char *name, int is_binary, char *buf, unsigned long size);
extern struct grep_opt *grep_opt_dup(const struct grep_opt *opt);
extern int grep_threads_ok(const struct grep_opt *opt);
diff --git a/revision.c b/revision.c
index c97d834..3dcd968 100644
--- a/revision.c
+++ b/revision.c
@@ -2150,6 +2150,7 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
return 1;
return grep_buffer(&opt->grep_filter,
NULL, /* we say nothing, not even filename */
+ -1,
commit->buffer, strlen(commit->buffer));
}
--
1.7.9.3.gc3fce1.dirty
^ permalink raw reply related
* Re: [PATCH] Don't search files with an unset "grep" attribute
From: Jeff King @ 2012-02-01 23:20 UTC (permalink / raw)
To: Conrad Irwin; +Cc: Junio C Hamano, git, Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <20120201221437.GA19044@sigill.intra.peff.net>
On Wed, Feb 01, 2012 at 05:14:37PM -0500, Jeff King wrote:
> > 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).
>
> I've been able to get somewhat better performance by hoisting the
> attribute lookup into the parent thread. That means it happens in order
> (which lets the attr code's stack optimizations work), and there's no
> lock contention.
>
> I'll post finished patches with numbers in a few minutes.
OK, here they are. After playing with some options, I'm satisfied this
is a sane way to do it. I don't think it's worth having a config option.
There is a measurable slowdown, but it's simply not that big.
[1/2]: grep: let grep_buffer callers specify a binary flag
[2/2]: grep: respect diff attributes for binary-ness
There are a few optimizations I didn't do that you could put on top:
1. When "-a" is given, we can avoid the attribute lookup altogether.
2. When "-I" is given, we can actually check attributes _before_
loading the file or blob into memory. This can help with very large
binaries.
3. When "-I" is given but we have no attribute, we can stream the
beginning of the file or blob to check for binary-ness, and then
avoid loading the whole thing if it turns out to be binary.
I think (1) and (2) should be easy. Doing (3) is a little messier,
because binary detection happens inside grep_buffer, but we can hoist it
out. However, for large files, it might be nice to have a streaming grep
interface anyway, and (3) could be part of that.
-Peff
^ permalink raw reply
* [RFC PATCH] gitweb: use CGI with -utf8
From: Michał Kiedrowicz @ 2012-02-01 22:50 UTC (permalink / raw)
To: git; +Cc: Michał Kiedrowicz
I noticed that gitweb tries a lot to properly process UTF-8 data, for
example it prints my name correctly in log and commit information, but
it echos junk in the search field. It looks like:
Michał Kiedrowicz
I don't know CGI well and I never touched gitewb code, but I found this
on http://www.lemoda.net/cgi/perl-unicode/index.html:
use CGI '-utf8';
my $value = params ('input');
I tried it and that fixed my problem. I'm not sure about the
consequences, maybe someone more experienced in CGI might help?
---
gitweb/gitweb.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index abb5a79..74d45b1 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -10,7 +10,7 @@
use 5.008;
use strict;
use warnings;
-use CGI qw(:standard :escapeHTML -nosticky);
+use CGI qw(:standard :escapeHTML -nosticky -utf8);
use CGI::Util qw(unescape);
use CGI::Carp qw(fatalsToBrowser set_message);
use Encode;
--
1.7.3.4
^ permalink raw reply related
* Re: Bug report: stash in upstream caused remote fetch to fail
From: Andrew Walrond @ 2012-02-01 22:37 UTC (permalink / raw)
To: Thomas Rast; +Cc: git
In-Reply-To: <874nvap9hj.fsf@thomas.inf.ethz.ch>
On Wed, Feb 01, 2012 at 09:46:48PM +0100, Thomas Rast wrote:
>
> Do you, by any chance, still have a copy of the upstream repo before you
> trashed the stash? It would be interesting to know whether there was
> actually some repository corruption going on (that went unnoticed by
> fsck, no less) or if there was a bug in the transmission.
I tried to reproduce the problem but without success :(
Disk corruption is unlikely - raided drives etc etc. More likely stash
related somehow as I was applying patches manually on older versions
checked out in a temp branch, stashing the results then applying the
stash onto master/HEAD.
But why was a remote clone involved with the upstream stash at all I
wonder??
Andrew Walrond
^ permalink raw reply
* Re: [RFC/PATCH] add update to branch support for "floating submodules"
From: Jens Lehmann @ 2012-02-01 22:37 UTC (permalink / raw)
To: Phil Hord; +Cc: Junio C Hamano, Leif Gruenwoldt, git
In-Reply-To: <CABURp0pSGGT8eyzNad-dNNx49oioAxOPOf3dmqu7M3fnV+PzdA@mail.gmail.com>
Am 31.01.2012 23:50, schrieb Phil Hord:
> What I mean is that a developer may be completely focused on one
> particular submodule (his domain). He does his work in this module,
> and when it's ready he commits and pushes to the server. 'git status'
> shows him that his directory is clean. But this is only because he
> doesn't really know where the submodules top-directories are, so he
> doesn't realize that he has changes in another submodule that he has
> not committed. He has to know to run 'git status' from somewhere in
> the superproject (ostensibly in the root directory of that
> superproject). But he may forget since 'git status' already assured
> him he was done.
<snip>
> I guess what would help here is something like the opposite of 'git
> status' showing the status of descendant submodules; it would help if
> it showed the status of sibling submodules and the superproject as
> well.
Hmm, I really think the fact that submodules are unaware that they
are part of a superproject is a feature. I'd prefer seeing that kind
of problem being tackled by the CI server and/or user education. Or
maybe a pre-commit hook which issues a warning in that case?
^ permalink raw reply
* Re: [PATCH v3 1/2] Factor find_pack_entry()'s core out
From: Junio C Hamano @ 2012-02-01 22:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nicolas Pitre, Nguyễn Thái Ngọc Duy, git
In-Reply-To: <7vaa522oum.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Nicolas Pitre <nico@fluxnic.net> writes:
>
>>> +static int find_pack_entry_1(const unsigned char *sha1,
>>> + struct packed_git *p, struct pack_entry *e)
>>
>> This looks all goot but the name. Pretty please, try to find something
>> that is more descriptive than "1". Suggestions:
>> "find_pack_entry_lookup", "find_pack_entry_inner", etc.
>
> Perhaps "find_pack_entry_in_pack(sha1, e, p)"?
> That would go well with the caller "find_pack_entry(sha1, e)".
I amended 1/2 (and adjusted 2/2 to match) to call it fill_pack_entry.
Will push out the result tonight.
^ permalink raw reply
* Request for git-svn man page
From: Dr. Lars Hanke @ 2012-02-01 22:32 UTC (permalink / raw)
To: git
Please add instructions to the git-svn man page concerning cloning a SVN
repositiory through a proxy. In particular make clear that also the SVN
software has to be configured to use the proxy. See e.g.
http://stackoverflow.com/questions/9057111/git-svn-ignores-http-proxy-debian.
I'd prepare a patch, but I have no clue about SVN, so I'm probably not
the best choice for writing a SVN related manual. ;)
Kind regards,
- lars.
^ permalink raw reply
* Re: General support for ! in git-config values
From: Junio C Hamano @ 2012-02-01 22:21 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Jeff King, Git Mailing List
In-Reply-To: <CACBZZX5mX55Rh8b2GYv7wKbCCypCkrn5AiM9BpXydgcYxHWdQA@mail.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>> I.e., everything pertaining to "!" happens after we get the config
>> string. So what is it that you want "git config --with-exec" to do?
>
> I agree that that's how it should work, I just suggested --with-exec
> in case anyone complained about the backwards compatibility issue of
> changing the meaning of "!" for existing configs.
Now you made me utterly confused.
What "backwards compatibility" issue do you have in mind? If I name
myself '!me' with "user.name = !me", do I suddenly break backwards
compatibility of Git with previous versions somehow? If so how?
The --with-exec option you talk about seems to me the option about the
backward compatibility of the _calling script_ of "git config". The old
version of git-blorl script may have used foo.bar as a mere string, but a
new version of it may (optionally) interpret foo.bar's value in a special
way when it begins with a "!", introducing a backward compatibility issues
to git-blorl script, and users who want to take advantage of that feature
may want to run it as "git-blorl --with-exec", and the relevant part of
the "git-blorl" script might look like this:
foo_bar=$(git config foo.bar)
case "$with_exec,$foo_bar" in
yes,\!*)
foo_bar=$(eval ${foo_bar#\!}) ;;
esac
... then do something interesting using $foo_bar ...
^ permalink raw reply
* Re: [PATCH] Don't search files with an unset "grep" attribute
From: Jeff King @ 2012-02-01 22:14 UTC (permalink / raw)
To: Conrad Irwin; +Cc: Junio C Hamano, git, Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <CAOTq_ptj06aNGsQRjV0fVRxnQFBHmU2FFSXwWDUUk9MM77k2LQ@mail.gmail.com>
On Wed, Feb 01, 2012 at 01:28:47AM -0800, Conrad Irwin wrote:
> > 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).
I've been able to get somewhat better performance by hoisting the
attribute lookup into the parent thread. That means it happens in order
(which lets the attr code's stack optimizations work), and there's no
lock contention.
I'll post finished patches with numbers in a few minutes.
> Tests from [1] included below in case they're still useful (they pass
> with your change)
Thanks, I'll include them.
-Peff
^ permalink raw reply
* Re: [PATCH v3 1/2] Factor find_pack_entry()'s core out
From: Junio C Hamano @ 2012-02-01 22:03 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <alpine.LFD.2.02.1202011056140.2759@xanadu.home>
Nicolas Pitre <nico@fluxnic.net> writes:
>> +static int find_pack_entry_1(const unsigned char *sha1,
>> + struct packed_git *p, struct pack_entry *e)
>
> This looks all goot but the name. Pretty please, try to find something
> that is more descriptive than "1". Suggestions:
> "find_pack_entry_lookup", "find_pack_entry_inner", etc.
Perhaps "find_pack_entry_in_pack(sha1, e, p)"?
That would go well with the caller "find_pack_entry(sha1, e)".
^ permalink raw reply
* Re: logging disjoint sets of commits in a single command
From: Bryan O'Sullivan @ 2012-02-01 22:01 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git@vger.kernel.org
In-Reply-To: <20120201030300.GA9969@sigill.intra.peff.net>
On 2012-01-31 19:03 , "Jeff King" <peff@peff.net> wrote:
>
>That sounds kind of slow. Is your repository really gigantic?
Bigger than a kernel tree, but not as many commits. Beyond that, can't say.
> Have you packed
>everything?
Yep.
> I'm just curious if there's some other way to make things
>faster.
It would be nice if there was, but the fundamental problem is the lack of
an index from filename to commit. Walking every commit is the limiting
factor, I believe.
> Is the repository publicly available?
No, sorry.
^ permalink raw reply
* AW: Project structure of .NET-Projects using git submodule or something different
From: Harald Heigl @ 2012-02-01 21:07 UTC (permalink / raw)
To: git
In-Reply-To: <4F29A0BE.8000803@web.de>
Hi, thanks for your answer!
> -----Ursprüngliche Nachricht-----
> Von: Jens Lehmann [mailto:Jens.Lehmann@web.de]
> Gesendet: Mittwoch, 01. Februar 2012 21:30
> An: Harald Heigl
> Cc: git@vger.kernel.org
> Betreff: Re: Project structure of .NET-Projects using git submodule or
> something different
>
> Am 31.01.2012 23:41, schrieb Harald Heigl:
> > Let's assume following Project structure (Dependencies and
> Subdependencies
> > are submodules and submodules of the submodules)
> > Project
> > Dependency 1
> > Dependency 2
> > Dependency 3
> > Dependency 4
> > Dependency 2
> >
> >
> > The problem is if I want to build them I need to build 2+3, then 1, 4
and 2
> > again and then the project. As you may see project 2 is a submodule of
> > dependency 1 and also of project. I don't feel comfortable with this
setup.
> > What do you think?
>
> Hmm, we try to avoid that kind of setup as having checked out different
> versions of the "Dependency 2" submodule could have rather surprising
> effects. We get along really well with "Dependency 2" only being present
> in the superproject and having "Dependency 1" reference that instead of
> having its own copy (So we have submodules which are depending on having
> other submodules right next to them). Then the superproject is responsible
> for tying it all together.
I think you're right, my first thoughts were that if I start a new project I
just "git submodule dependency1" and get all the required dependencies and
the dependencies within the dependencies and so on ... .
With your solution I "git submodule dependency1" and have to think about the
dependencies it depends on. On the other hand we are just a small company
and the number of submodules is not too big and the missing references in a
new project would be easily identifiable, so ... .
And if I want to checkout dependency 1 individually (for whatever reason), I
could still do something like this:
SuperDependency1 (with solution-File)
Dependency1 (as submodule)
Dependency2 (dependency of dependency1 - as submodule)
Dependency3 (dependency of dependency1 - as submodule)
Thanks again, I see my concept causes some trouble ...
Any other thoughts or other workflows with git or with tools build around
git?
Thanks again,
Harald
^ permalink raw reply
* rebase -i reword runs pre-commit hook with curious results
From: Neal Kreitzinger @ 2012-02-01 21:50 UTC (permalink / raw)
To: git
I'm confused on why and/or how interactive rebase runs the pre-commit hook
when doing the reword command for commit (a). My pre-commit hook does
keyword expansion on the worktree copy of the modified index files and then
re-adds them to effect a user-date-stamp when committed. However, the
user-date-stamps don't get updated when reword runs the pre-commit hook.
IOW, the pre-commit hook does not get the same results as if I were doing a
commandline git-commit of a modified index. I suppose reword is protecting
the preservation of all the contents of commit (a) except the commit message
which makes sense, but I don't understand how it goes about doing this while
still attempting to somehow honor the pre-commit hook. (git 1.7.1)
v/r,
neal
^ permalink raw reply
* Re: rebase -i reword converts to pick on pre-commit non-zero exit
From: Neal Kreitzinger @ 2012-02-01 21:28 UTC (permalink / raw)
To: git; +Cc: git
In-Reply-To: <jgcaoh$d9q$1@dough.gmane.org>
On 2/1/2012 3:27 PM, Neal Kreitzinger wrote:
> When interactive rebase does the reword command it runs the pre-commit hook
> for that commit (a). If the pre-commit hook gives a non-zero exit then
> interactive rebase picks commit (a) and continues to the next commit (b) in
> the rebase-to-do-list. 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? Otherwise, you have to run another rebase after the
> rejected rewords and edit those commits to accomplish the rewords.
>
git 1.7.1
v/r,
neal
^ permalink raw reply
* rebase -i reword converts to pick on pre-commit non-zero exit
From: Neal Kreitzinger @ 2012-02-01 21:27 UTC (permalink / raw)
To: git
When interactive rebase does the reword command it runs the pre-commit hook
for that commit (a). If the pre-commit hook gives a non-zero exit then
interactive rebase picks commit (a) and continues to the next commit (b) in
the rebase-to-do-list. 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? Otherwise, you have to run another rebase after the
rejected rewords and edit those commits to accomplish the rewords.
v/r,
neal
^ permalink raw reply
* Re: [PATCH v2] Use correct grammar in diffstat summary line
From: Junio C Hamano @ 2012-02-01 21:26 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, Thomas Dickey, Jonathan Nieder, Ævar Arnfjörð,
Frederik Schwarzer, Brandon Casey, dickey
In-Reply-To: <1328100907-20397-1-git-send-email-pclouds@gmail.com>
Nice. Will queue with a minor update to the log message:
commit 3f29ab34372ee11946439da3bde307eb90ad9031
Author: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date: Wed Feb 1 19:55:07 2012 +0700
Use correct grammar in diffstat summary line
"git diff --stat" and "git apply --stat" now learn to print the line
"%d files changed, %d insertions(+), %d deletions(-)" in singular form
whenever applicable. "0 insertions" and "0 deletions" are also omitted
unless they are both zero.
This matches how versions of "diffstat" that are not prehistoric produced
their output, and also makes this line translatable.
[jc: with help from Thomas Dickey in archaeology of "diffstat"]
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
And also this bit on top:
diff --git a/diff.c b/diff.c
index 5c31b36..5f3ce97 100644
--- a/diff.c
+++ b/diff.c
@@ -1329,7 +1329,7 @@ int print_stat_summary(FILE *fp, int files, int insertions, int deletions)
if (!files) {
assert(insertions == 0 && deletions == 0);
- return fputs(_(" no changes\n"), fp);
+ return fputs(_(" 0 files changed\n"), fp);
}
strbuf_addf(&sb,
^ permalink raw reply related
* Re: General support for ! in git-config values
From: Ævar Arnfjörð Bjarmason @ 2012-02-01 21:25 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List
In-Reply-To: <20120201184020.GA29374@sigill.intra.peff.net>
On Wed, Feb 1, 2012 at 19:40, Jeff King <peff@peff.net> wrote:
> On Wed, Feb 01, 2012 at 06:33:47PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> For a program I'm working on (git-deploy) I'd like to have this as a
>> general facility, i.e. users can specify either:
>>
>> foo.bar = value
>>
>> Or:
>>
>> foo.bar = !cat /some/path
>>
>> I'm wondering why git-config doesn't do this already, if there's no
>> reason in particular I can just patch it in, either as a new option:
>>
>> git config --with-exec --get foo.bar
>
> I'm not clear on what you want --with-exec to do. By default, config
> values are strings. I would expect the "!" to be a special marker that
> the caller would recognize in the string, and then act appropriately.
>
> So if I were implementing git aliases in the shell, the code would look
> like:
>
> v=$(git config alias.$alias)
> case "$v" in
> "")
> die "no such alias: $alias" ;;
> "!*)
> cmd="${v#!}" ;;
> *)
> cmd="git $v" ;;
> esac
> eval "$cmd"
>
> I.e., everything pertaining to "!" happens after we get the config
> string. So what is it that you want "git config --with-exec" to do?
I agree that that's how it should work, I just suggested --with-exec
in case anyone complained about the backwards compatibility issue of
changing the meaning of "!" for existing configs.
^ permalink raw reply
* Re: [PATCH v5 2/5] gitweb: add project_filter to limit project list to a subdirectory
From: Junio C Hamano @ 2012-02-01 20:55 UTC (permalink / raw)
To: Bernhard R. Link; +Cc: Jakub Narebski, git
In-Reply-To: <20120201165902.GA14706@server.brlink.eu>
"Bernhard R. Link" <brl+git@mail.brlink.eu> writes:
> Look liks a change like that is actually needed...
> ... So it looks for forks in a directory named '1'
Yeah, that was exactly what was causing failures in 9502. Fixed locally
so no further action is required.
Thanks.
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2012, #08; Tue, 31)
From: Junio C Hamano @ 2012-02-01 20:46 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Bernhard R. Link
In-Reply-To: <7v39au47dt.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> I am getting
>
> 7ad1b64084ff003f71fe749a3e5a74d071a193d8 is the first bad commit
> commit 7ad1b64084ff003f71fe749a3e5a74d071a193d8
> Author: Bernhard R. Link <brl+git@mail.brlink.eu>
> Date: Mon Jan 30 21:05:47 2012 +0100
>
> gitweb: move hard coded .git suffix out of git_get_projects_list
>
> *** t9502-gitweb-standalone-parse-output.sh ***
> ...
> not ok - 14 forks: "forks" action for forked repository
> #
> # gitweb_run "p=foo.git;a=forks" &&
> # grep -q ">foo/foo-forked\\.git<" gitweb.body &&
> # grep -q ">fork of foo<" gitweb.body
> #
> ok 15 - forks: can access forked repository
> ok 16 - forks: project_index lists all projects (incl. forks)
> # failed 1 among 16 test(s)
>
> The output file gitweb.body has this in it:
>
> <div class="page_body">
> <br /><br />
> 404 - No forks found
> <br />
> </div>
> <div class="page_footer">
And of course the culprit turns out to be that "cute" expression.
-- >8 --
Subject: gitweb: do not use assignment with regexp replace in parameter
A recent patch made the code to generate a parameter to git_get_projects_list
a bit too cute, by introducing a new variable, assigning a value to it, and
then munging that value with s/// replacement, all in the parameter list.
The whole expression returns the number of replacements, not the resulting
value in the variable after s/// operation.
Split them into separate expressions, which also would make the resulting
lines shorter and less taxing on the brain.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
gitweb/gitweb.perl | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b764d51..e074cd7 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -6003,7 +6003,9 @@ sub git_forks {
die_error(400, "Unknown order parameter");
}
- my @list = git_get_projects_list((my $filter = $project) =~ s/\.git$//);
+ my $filter = $project;
+ $filter =~ s/\.git$//;
+ my @list = git_get_projects_list($filter);
if (!@list) {
die_error(404, "No forks found");
}
@@ -6062,7 +6064,9 @@ sub git_summary {
if ($check_forks) {
# find forks of a project
- @forklist = git_get_projects_list((my $filter = $project) =~ s/\.git$//);
+ my $filter = $project;
+ $filter =~ s/\.git$//;
+ @forklist = git_get_projects_list($filter);
# filter out forks of forks
@forklist = filter_forks_from_projects_list(\@forklist)
if (@forklist);
^ permalink raw reply related
* Re: What's cooking in git.git (Jan 2012, #08; Tue, 31)
From: Junio C Hamano @ 2012-02-01 20:37 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Bernhard R. Link
In-Reply-To: <m3fweudaf6.fsf@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> writes:
> Hmmm... strange. I have applied my patches on top of earlier version
> of project_filter commits:
>
> - gitweb: Make project search respect project_filter
> - gitweb: Improve projects search form
> - gitweb: place links to parent directories in page header
> - gitweb: add project_filter to limit project list to a subdirectory
>
> and all gitweb tests passes.
>
> I will investigate.
I am getting
7ad1b64084ff003f71fe749a3e5a74d071a193d8 is the first bad commit
commit 7ad1b64084ff003f71fe749a3e5a74d071a193d8
Author: Bernhard R. Link <brl+git@mail.brlink.eu>
Date: Mon Jan 30 21:05:47 2012 +0100
gitweb: move hard coded .git suffix out of git_get_projects_list
*** t9502-gitweb-standalone-parse-output.sh ***
ok 1 - setup
ok 2 - snapshot: full sha1
ok 3 - snapshot: shortened sha1
ok 4 - snapshot: almost full sha1
ok 5 - snapshot: HEAD
ok 6 - snapshot: short branch name (master)
ok 7 - snapshot: short tag name (first)
ok 8 - snapshot: full branch name (refs/heads/master)
ok 9 - snapshot: full tag name (refs/tags/first)
ok 10 - snapshot: hierarchical branch name (xx/test)
ok 11 - forks: setup
ok 12 - forks: not skipped unless "forks" feature enabled
ok 13 - forks: forks skipped if "forks" feature enabled
not ok - 14 forks: "forks" action for forked repository
#
# gitweb_run "p=foo.git;a=forks" &&
# grep -q ">foo/foo-forked\\.git<" gitweb.body &&
# grep -q ">fork of foo<" gitweb.body
#
ok 15 - forks: can access forked repository
ok 16 - forks: project_index lists all projects (incl. forks)
# failed 1 among 16 test(s)
The output file gitweb.body has this in it:
<div class="page_body">
<br /><br />
404 - No forks found
<br />
</div>
<div class="page_footer">
^ permalink raw reply
* Re: Project structure of .NET-Projects using git submodule or something different
From: Jens Lehmann @ 2012-02-01 20:29 UTC (permalink / raw)
To: Harald Heigl; +Cc: git
In-Reply-To: <002401cce069$75ecc1a0$61c644e0$@heigl-online.at>
Am 31.01.2012 23:41, schrieb Harald Heigl:
> Let's assume following Project structure (Dependencies and Subdependencies
> are submodules and submodules of the submodules)
> Project
> Dependency 1
> Dependency 2
> Dependency 3
> Dependency 4
> Dependency 2
>
>
> The problem is if I want to build them I need to build 2+3, then 1, 4 and 2
> again and then the project. As you may see project 2 is a submodule of
> dependency 1 and also of project. I don't feel comfortable with this setup.
> What do you think?
Hmm, we try to avoid that kind of setup as having checked out different
versions of the "Dependency 2" submodule could have rather surprising
effects. We get along really well with "Dependency 2" only being present
in the superproject and having "Dependency 1" reference that instead of
having its own copy (So we have submodules which are depending on having
other submodules right next to them). Then the superproject is responsible
for tying it all together.
^ permalink raw reply
* Bug report: stash in upstream caused remote fetch to fail
From: Andrew Walrond @ 2012-02-01 16:59 UTC (permalink / raw)
To: andrew
A bit of cut and paste will explain better than me...
LOCAL $ git remote update
Fetching origin
remote: Counting objects: 25, done.
remote: Compressing objects: 100% (12/12), done.
remote: Total 13 (delta 10), reused 0 (delta 0)
Unpacking objects: 100% (13/13), done.
fatal: bad object fa0da15b2ea5cc3e4eb9ed414b99d6a9d7da7864
error: git://git.heresymail.org/lib%2Fmpfr did not send all necessary
objects
UPSTREAM $ git fsck
dangling blob ded848b21db04fcadf77a4a5d9f81955b4315c9f
dangling blob 9c3976919b3cee56eabc3c9c9dfe5d223ce32686
dangling blob e17ab25a3a91bed830ddb06da4af1132434d5ee4
dangling blob 20a612ab361058838f680d72c1f4f8cb462ce1a2
UPSTREAM $ git gc
Counting objects: 974, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (954/954), done.
Writing objects: 100% (974/974), done.
Total 974 (delta 572), reused 0 (delta 0)
LOCAL $ git remote update
Fetching origin
remote: Counting objects: 25, done.
remote: Compressing objects: 100% (10/10), done.
remote: Total 13 (delta 3), reused 11 (delta 2)
Unpacking objects: 100% (13/13), done.
fatal: bad object fa0da15b2ea5cc3e4eb9ed414b99d6a9d7da7864
error: git://git.heresymail.org/lib%2Fmpfr did not send all necessary
objects
LOCAL $ DELETE LOCAL REPO
LOCAL $ git clone <upstream>
Cloning into bare repository '/src/lib/mpfr'...
remote: Counting objects: 972, done.
remote: Compressing objects: 100% (382/382), done.
remote: Total 972 (delta 570), reused 971 (delta 570)
Receiving objects: 100% (972/972), 2.01 MiB, done.
Resolving deltas: 100% (570/570), done.
error: refs/stash does not point to a valid object!
UPSTREAM $ git stash list
stash@{0}: WIP on (no branch): f648dd0 Import 3.1.0 from tarball
UPSTREAM $ git stash clear
LOCAL $ DELETE LOCAL REPO
LOCAL $ git clone <upstream>
Cloning into bare repository '/src/lib/mpfr'...
remote: Counting objects: 972, done.
remote: Compressing objects: 100% (382/382), done.
remote: Total 972 (delta 570), reused 971 (delta 570)
Receiving objects: 100% (972/972), 2.01 MiB, done.
Resolving deltas: 100% (570/570), done.
UPSTREAM $ git version
git version 1.7.8.2
LOCAL $ git version
git version 1.7.8.2
Hope that's useful!
Andrew Walrond
^ permalink raw reply
* Re: Rebase regression in v1.7.9?
From: Felipe Contreras @ 2012-02-01 19:30 UTC (permalink / raw)
To: Andrew Wong; +Cc: git
In-Reply-To: <4F29761E.1030605@sohovfx.com>
On Wed, Feb 1, 2012 at 7:27 PM, Andrew Wong <andrew.w@sohovfx.com> wrote:
> On 01/31/2012 05:56 PM, Felipe Contreras wrote:
>> The rebase will finish, but there will be a .git/CHERRY_PICK_HEAD file.
>>
> Ah, good catch. I can reproduce the issue. This is only happening in
> "rebase -i" because interactive rebase relies on cherry-pick, but not
> regular rebase. And now cherry-pick creates a state when there's a
> conflict (since 1.7.5?), which "rebase -i" didn't expect before. We
> probably just need to do a manual clean up before "rebase -i" continues.
>
> I'll try to come up with a patch for this. In the mean time, doing a
> "git reset" will remove that dangling file. Of course, you could always
> manually remove it. Does the dangling file cause a subsequent git
> command to fail?
No, it's just annoying with the __git_ps1 prompt stuff. Yeah, 'git
reset' solves the problem, but it's much better to type 'git skip'
instead... for now.
--
Felipe Contreras
^ permalink raw reply
* Re: General support for ! in git-config values
From: Jeff King @ 2012-02-01 18:40 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List
In-Reply-To: <CACBZZX6U+1Fmdaz2ikbbc6zUyF=pMGQOqUGVOWCkUFBUkovCBw@mail.gmail.com>
On Wed, Feb 01, 2012 at 06:33:47PM +0100, Ævar Arnfjörð Bjarmason wrote:
> For a program I'm working on (git-deploy) I'd like to have this as a
> general facility, i.e. users can specify either:
>
> foo.bar = value
>
> Or:
>
> foo.bar = !cat /some/path
>
> I'm wondering why git-config doesn't do this already, if there's no
> reason in particular I can just patch it in, either as a new option:
>
> git config --with-exec --get foo.bar
I'm not clear on what you want --with-exec to do. By default, config
values are strings. I would expect the "!" to be a special marker that
the caller would recognize in the string, and then act appropriately.
So if I were implementing git aliases in the shell, the code would look
like:
v=$(git config alias.$alias)
case "$v" in
"")
die "no such alias: $alias" ;;
"!*)
cmd="${v#!}" ;;
*)
cmd="git $v" ;;
esac
eval "$cmd"
I.e., everything pertaining to "!" happens after we get the config
string. So what is it that you want "git config --with-exec" to do?
-Peff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox