* Re: Something is broken in repack
From: Paolo Bonzini @ 2007-12-12 16:17 UTC (permalink / raw)
To: gcc; +Cc: git
In-Reply-To: <alpine.LFD.0.99999.0712120743040.555@xanadu.home>
> When I returned to the computer this morning, the repack was
> completed... with a 1.3GB pack instead.
>
> So... The gcc repo apparently really needs a large window to efficiently
> compress those large objects.
So, am I right that if you have a very well-done pack (such as gcc's),
you might want to repack in two phases:
- first discarding the old deltas and using a small window, thus
producing a bad pack that can be repacked without humongous amounts of
memory...
- ... then discarding the old deltas and producing another
well-compressed pack?
Paolo
^ permalink raw reply
* [PATCH 2/4] Extract and improve whitespace check from "git apply"
From: Wincent Colaiuta @ 2007-12-12 16:23 UTC (permalink / raw)
To: git; +Cc: gitster, Wincent Colaiuta
In-Reply-To: <1197476582-18956-2-git-send-email-win@wincent.com>
This refactoring extracts the whitespace checking formerly done in
builtin-apply.c into a function in ws.c so that the exact same
whitespace logic can be used by "git diff" as well, thus reducing
code duplication and the likelihood of errors.
At the same time we teach "git apply" to report all classes of
whitespace errors found in a given line rather than reporting only
the first found error.
Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
builtin-apply.c | 60 +++++++++++-------------------------------------------
cache.h | 2 +
ws.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 73 insertions(+), 47 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index f2e9a33..cf349db 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -898,58 +898,24 @@ static int find_header(char *line, unsigned long size, int *hdrsize, struct patc
return -1;
}
-static void check_whitespace(const char *line, int len, unsigned ws_rule)
+static void check_patch_whitespace(const char *line, int len,
+ unsigned ws_rule)
{
- const char *err = "Adds trailing whitespace";
- int seen_space = 0;
- int i;
-
- /*
- * We know len is at least two, since we have a '+' and we
- * checked that the last character was a '\n' before calling
- * this function. That is, an addition of an empty line would
- * check the '+' here. Sneaky...
- */
- if ((ws_rule & WS_TRAILING_SPACE) && isspace(line[len-2]))
- goto error;
-
- /*
- * Make sure that there is no space followed by a tab in
- * indentation.
- */
- if (ws_rule & WS_SPACE_BEFORE_TAB) {
- err = "Space in indent is followed by a tab";
- for (i = 1; i < len; i++) {
- if (line[i] == '\t') {
- if (seen_space)
- goto error;
- }
- else if (line[i] == ' ')
- seen_space = 1;
- else
- break;
- }
- }
-
- /*
- * Make sure that the indentation does not contain more than
- * 8 spaces.
- */
- if ((ws_rule & WS_INDENT_WITH_NON_TAB) &&
- (8 < len) && !strncmp("+ ", line, 9)) {
- err = "Indent more than 8 places with spaces";
- goto error;
- }
- return;
+ char *err;
+ unsigned result = check_whitespace(line, len, ws_rule);
+ if (!result)
+ return;
- error:
whitespace_error++;
if (squelch_whitespace_errors &&
squelch_whitespace_errors < whitespace_error)
;
- else
+ else {
+ err = whitespace_error_string(result);
fprintf(stderr, "%s.\n%s:%d:%.*s\n",
- err, patch_input_file, linenr, len-2, line+1);
+ err, patch_input_file, linenr, len - 2, line + 1);
+ free(err);
+ }
}
/*
@@ -1001,7 +967,7 @@ static int parse_fragment(char *line, unsigned long size,
case '-':
if (apply_in_reverse &&
ws_error_action != nowarn_ws_error)
- check_whitespace(line, len, patch->ws_rule);
+ check_patch_whitespace(line, len, patch->ws_rule);
deleted++;
oldlines--;
trailing = 0;
@@ -1009,7 +975,7 @@ static int parse_fragment(char *line, unsigned long size,
case '+':
if (!apply_in_reverse &&
ws_error_action != nowarn_ws_error)
- check_whitespace(line, len, patch->ws_rule);
+ check_patch_whitespace(line, len, patch->ws_rule);
added++;
newlines--;
trailing = 0;
diff --git a/cache.h b/cache.h
index 1bcb3df..6c17f27 100644
--- a/cache.h
+++ b/cache.h
@@ -655,6 +655,8 @@ void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, i
extern unsigned whitespace_rule_cfg;
extern unsigned whitespace_rule(const char *);
extern unsigned parse_whitespace_rule(const char *);
+extern unsigned check_whitespace(const char *line, int len, unsigned ws_rule);
+extern char *whitespace_error_string(unsigned ws);
/* ls-files */
int pathspec_match(const char **spec, char *matched, const char *filename, int skiplen);
diff --git a/ws.c b/ws.c
index 52c10ca..884d373 100644
--- a/ws.c
+++ b/ws.c
@@ -94,3 +94,61 @@ unsigned whitespace_rule(const char *pathname)
return whitespace_rule_cfg;
}
}
+
+/* The returned string should be freed by the caller. */
+char *whitespace_error_string(unsigned ws)
+{
+ struct strbuf err;
+ strbuf_init(&err, 0);
+ if (ws & WS_TRAILING_SPACE)
+ strbuf_addstr(&err, "Adds trailing whitespace");
+ if (ws & WS_SPACE_BEFORE_TAB) {
+ if (err.len)
+ strbuf_addstr(&err, ", ");
+ strbuf_addstr(&err, "Space in indent is followed by a tab");
+ }
+ if (ws & WS_INDENT_WITH_NON_TAB) {
+ if (err.len)
+ strbuf_addstr(&err, ", ");
+ strbuf_addstr(&err, "Indent more than 8 places with spaces");
+ }
+ return strbuf_detach(&err, NULL);
+}
+
+unsigned check_whitespace(const char *line, int len, unsigned ws_rule)
+{
+ unsigned result = 0;
+ int seen_space = 0;
+ int i;
+
+ if (ws_rule & WS_TRAILING_SPACE) {
+ /* Lines start with "+" or "-" so length is at least 1 */
+ if (line[len - 1] == '\n') {
+ if (isspace(line[len - 2]))
+ result |= WS_TRAILING_SPACE;
+ }
+ else if (isspace(line[len - 1]))
+ result |= WS_TRAILING_SPACE;
+ }
+
+ if (ws_rule & WS_SPACE_BEFORE_TAB) {
+ for (i = 1; i < len; i++) {
+ if (line[i] == '\t') {
+ if (seen_space) {
+ result |= WS_SPACE_BEFORE_TAB;
+ break;
+ }
+ }
+ else if (line[i] == ' ')
+ seen_space = 1;
+ else
+ break;
+ }
+ }
+
+ if ((ws_rule & WS_INDENT_WITH_NON_TAB) &&
+ (8 < len) && !strncmp("+ ", line, 9)) {
+ result |= WS_INDENT_WITH_NON_TAB;
+ }
+ return result;
+}
--
1.5.3.7.1159.g2f071-dirty
^ permalink raw reply related
* [PATCH 4/4] Add tests for "git diff --check" with core.whitespace options
From: Wincent Colaiuta @ 2007-12-12 16:23 UTC (permalink / raw)
To: git; +Cc: gitster, Wincent Colaiuta
In-Reply-To: <1197476582-18956-4-git-send-email-win@wincent.com>
Make sure that "git diff --check" does the right thing when the
core.whitespace options are set.
Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
t/t4015-diff-whitespace.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 48 insertions(+), 0 deletions(-)
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index ff77a16..8d980af 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -256,4 +256,52 @@ test_expect_failure 'check mixed spaces and tabs in indent' '
'
+test_expect_success 'check trailing whitespace (trailing-space: off)' '
+
+ git config core.whitespace "-trailing-space" &&
+ echo "foo (); " > x &&
+ git diff --check
+
+'
+
+test_expect_failure 'check trailing whitespace (trailing-space: on)' '
+
+ git config core.whitespace "trailing-space" &&
+ echo "foo (); " > x &&
+ git diff --check
+
+'
+
+test_expect_success 'check space before tab in indent (space-before-tab: off)' '
+
+ git config core.whitespace "-space-before-tab" &&
+ echo " foo ();" > x &&
+ git diff --check
+
+'
+
+test_expect_failure 'check space before tab in indent (space-before-tab: on)' '
+
+ git config core.whitespace "space-before-tab" &&
+ echo " foo (); " > x &&
+ git diff --check
+
+'
+
+test_expect_success 'check spaces as indentation (indent-with-non-tab: off)' '
+
+ git config core.whitespace "-indent-with-non-tab"
+ echo " foo ();" > x &&
+ git diff --check
+
+'
+
+test_expect_failure 'check spaces as indentation (indent-with-non-tab: on)' '
+
+ git config core.whitespace "indent-with-non-tab" &&
+ echo " foo ();" > x &&
+ git diff --check
+
+'
+
test_done
--
1.5.3.7.1159.g2f071-dirty
^ permalink raw reply related
* [PATCH 1/4] Fix "diff --check" whitespace detection
From: Wincent Colaiuta @ 2007-12-12 16:22 UTC (permalink / raw)
To: git; +Cc: gitster, Wincent Colaiuta
In-Reply-To: <1197476582-18956-1-git-send-email-win@wincent.com>
"diff --check" would only detect spaces before tabs if a tab was the
last character in the leading indent. Fix that and add a test case to
make sure the bug doesn't regress in the future.
Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
Although this patch is made to apply on top of the topic I posted
earlier, it can be applied to master with a couple of tweaks. Let
me know if you want me to send such a patch.
In any case, [2/4] and [3/4] factor away the code that had the bug
in it anyway...
diff.c | 13 ++++++++++---
t/t4015-diff-whitespace.sh | 8 ++++++++
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/diff.c b/diff.c
index e89c7ce..c9b3884 100644
--- a/diff.c
+++ b/diff.c
@@ -1010,11 +1010,18 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
int i, spaces = 0, space_before_tab = 0, white_space_at_end = 0;
/* check space before tab */
- for (i = 1; i < len && (line[i] == ' ' || line[i] == '\t'); i++)
+ for (i = 1; i < len; i++) {
if (line[i] == ' ')
spaces++;
- if (line[i - 1] == '\t' && spaces)
- space_before_tab = 1;
+ else if (line[i] == '\t') {
+ if (spaces) {
+ space_before_tab = 1;
+ break;
+ }
+ }
+ else
+ break;
+ }
/* check whitespace at line end */
if (line[len - 1] == '\n')
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 5aaf2db..ff77a16 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -248,4 +248,12 @@ test_expect_failure 'check with space before tab in indent (diff-tree)' '
'
+# was a bug: space before tab only caught if tab was last in the indent
+test_expect_failure 'check mixed spaces and tabs in indent' '
+
+ echo " foo();" > x &&
+ git diff --check
+
+'
+
test_done
--
1.5.3.7.1159.g2f071-dirty
^ permalink raw reply related
* [PATCH 0/4] Refactoring for apply and diff
From: Wincent Colaiuta @ 2007-12-12 16:22 UTC (permalink / raw)
To: git; +Cc: gitster
4 more patches which apply on top of the "git diff --check" topic I
posted previously. If you'd prefer any of these to be prepared for
application on top of "master" or "next" let me know.
[1/4] Fix "diff --check" whitespace detection
Fix bug noticed while comparing the whitespace checks in "git apply"
and "git diff". It's exactly this kind of bug which motivates the
refactoring which follows...
[2/4] Extract and improve whitespace check from "git apply"
Extract (and improve) some stuff out of "apply"...
[3/4] Make "diff --check" use shared whitespace functions
... so that the same code can be used by "diff".
[4/4] Add tests for "git diff --check" with core.whitespace options
Cheers,
Wincent
^ permalink raw reply
* [PATCH 3/3] Make "diff --check" use shared whitespace functions
From: Wincent Colaiuta @ 2007-12-12 16:23 UTC (permalink / raw)
To: git; +Cc: gitster, Wincent Colaiuta
In-Reply-To: <1197476582-18956-3-git-send-email-win@wincent.com>
Use the logic refactored into ws.c rather than duplicating it.
Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
diff.c | 49 +++++++++++--------------------------------------
1 files changed, 11 insertions(+), 38 deletions(-)
diff --git a/diff.c b/diff.c
index c9b3884..7ad3c63 100644
--- a/diff.c
+++ b/diff.c
@@ -996,7 +996,7 @@ struct checkdiff_t {
const char *filename;
int lineno, color_diff;
unsigned ws_rule;
- int status;
+ unsigned status;
};
static void checkdiff_consume(void *priv, char *line, unsigned long len)
@@ -1005,45 +1005,18 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
const char *ws = diff_get_color(data->color_diff, DIFF_WHITESPACE);
const char *reset = diff_get_color(data->color_diff, DIFF_RESET);
const char *set = diff_get_color(data->color_diff, DIFF_FILE_NEW);
+ char *err;
if (line[0] == '+') {
- int i, spaces = 0, space_before_tab = 0, white_space_at_end = 0;
-
- /* check space before tab */
- for (i = 1; i < len; i++) {
- if (line[i] == ' ')
- spaces++;
- else if (line[i] == '\t') {
- if (spaces) {
- space_before_tab = 1;
- break;
- }
- }
- else
- break;
- }
-
- /* check whitespace at line end */
- if (line[len - 1] == '\n')
- len--;
- if (isspace(line[len - 1]))
- white_space_at_end = 1;
-
- if (space_before_tab || white_space_at_end) {
- data->status = 1;
- printf("%s:%d: %s", data->filename, data->lineno, ws);
- if (space_before_tab) {
- printf("space before tab");
- if (white_space_at_end)
- putchar(',');
- }
- if (white_space_at_end)
- printf("whitespace at end");
- printf(":%s ", reset);
- emit_line_with_ws(1, set, reset, ws, line, len,
- data->ws_rule);
- }
-
+ data->status = check_whitespace(line, len, data->ws_rule);
+ if (!data->status)
+ return;
+ err = whitespace_error_string(data->status);
+ printf("%s:%d: %s%s:%s ", data->filename, data->lineno,
+ ws, err, reset);
+ free(err);
+ emit_line_with_ws(1, set, reset, ws, line, len,
+ data->ws_rule);
data->lineno++;
} else if (line[0] == ' ')
data->lineno++;
--
1.5.3.7.1159.g2f071-dirty
^ permalink raw reply related
* Re: Efficient retrieval of commit log info
From: Linus Torvalds @ 2007-12-12 16:24 UTC (permalink / raw)
To: Eirik Bj?rsn?s; +Cc: git
In-Reply-To: <34660cca0712120636w149e2a82h84609f8ac7c958a9@mail.gmail.com>
On Wed, 12 Dec 2007, Eirik Bj?rsn?s wrote:
>
> Is it at all possible using the git network protocols to fetch just
> the commit log info, without transferring the contents?
>From a _protocol_ angle that's trivial (the git network protocol doesn't
really care what it transfers, it just transfers objects), but no, we
don't actually expose any capability like that. If you were to just want
this privately, the quick hack is to just disable traversal of trees in
builtin-pack-objects on the server side, but it sounds like you want to do
this in general (not just with git and not just with a server you
control), and if so, you're kind of screwed.
You could hack around it (very inefficiently) by parsing gitweb output, of
course, but I bet that you'd probably get better performance just cloning
the whole thing (which allows streaming, rather than trying to click
through commits on gitweb)
Linus
^ permalink raw reply
* Re: [(not so) random thoughts] using git as its own caching tool
From: Pierre Habouzit @ 2007-12-12 16:27 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Git ML
In-Reply-To: <475FFFB7.4010102@op5.se>
[-- Attachment #1: Type: text/plain, Size: 1153 bytes --]
On Wed, Dec 12, 2007 at 03:35:19PM +0000, Andreas Ericsson wrote:
> Pierre Habouzit wrote:
> > So am I having crazy thoughts and should I throw my crack-pipe away ?
> >Or does parts of this mumbling makes any sense to someone ?
>
> A bit of both ;-)
>
> I like the idea to use the git object store, because that certainly
> has an API that can't be done away with by user config. The reflog
> and its expiration mechanism is subject to human control though, and
> everyone doesn't even have them enabled. I don't for some repos where
> I know I'll create a thousand-and-one loose objects by rebasing,
> --amend'ing and otherwise fiddling with history rewrites.
>
> Having a tool that works on some repos but not on others because it
> relies on me living with an auto-gc after pretty much every operation
> would be very tiresome indeed.
Well if you disable the reflog on some repositories, those commands
will just be slow. But would still work.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: Something is broken in repack
From: Linus Torvalds @ 2007-12-12 16:37 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Jon Smirl, Junio C Hamano, gcc, Git Mailing List
In-Reply-To: <alpine.LFD.0.99999.0712120743040.555@xanadu.home>
On Wed, 12 Dec 2007, Nicolas Pitre wrote:
>
> So... my conclusion is that the glibc allocator has fragmentation issues
> with this work load, given the notable difference with the Google
> allocator, which itself might not be completely immune to fragmentation
> issues of its own.
Yes.
Note that delta following involves patterns something like
allocate (small) space for delta
for i in (1..depth) {
allocate large space for base
allocate large space for result
.. apply delta ..
free large space for base
free small space for delta
}
so if you have some stupid heap algorithm that doesn't try to merge and
re-use free'd spaces very aggressively (because that takes CPU time!), you
might have memory usage be horribly inflated by the heap having all those
holes for all the objects that got free'd in the chain that don't get
aggressively re-used.
Threaded memory allocators then make this worse by probably using totally
different heaps for different threads (in order to avoid locking), so they
will *all* have the fragmentation issue.
And if you *really* want to cause trouble for a memory allocator, what you
should try to do is to allocate the memory in one thread, and free it in
another, and then things can really explode (the freeing thread notices
that the allocation is not in its thread-local heap, so instead of really
freeing it, it puts it on a separate list of areas to be freed later by
the original thread when it needs memory - or worse, it adds it to the
local thread list, and makes it effectively totally impossible to then
ever merge different free'd allocations ever again because the freed
things will be on different heap lists!).
I'm not saying that particular case happens in git, I'm just saying that
it's not unheard of. And with the delta cache and the object lookup, it's
not at _all_ impossible that we hit the "allocate in one thread, free in
another" case!
Linus
^ permalink raw reply
* Re: Something is broken in repack
From: David Miller @ 2007-12-12 16:42 UTC (permalink / raw)
To: torvalds; +Cc: nico, jonsmirl, gitster, gcc, git
In-Reply-To: <alpine.LFD.0.9999.0712120826440.25032@woody.linux-foundation.org>
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Wed, 12 Dec 2007 08:37:10 -0800 (PST)
> I'm not saying that particular case happens in git, I'm just saying that
> it's not unheard of. And with the delta cache and the object lookup, it's
> not at _all_ impossible that we hit the "allocate in one thread, free in
> another" case!
One thing that supports these theories is that, while running
these large repacks, I notice that the RSS is roughly 2/3 of
the amount of virtual address space allocated.
I personally don't think it's unreasonable for GIT to have it's
own customized allocator at least for certain object types.
^ permalink raw reply
* Re: Something is broken in repack
From: Linus Torvalds @ 2007-12-12 16:54 UTC (permalink / raw)
To: David Miller; +Cc: nico, jonsmirl, gitster, gcc, git
In-Reply-To: <20071212.084212.02518392.davem@davemloft.net>
On Wed, 12 Dec 2007, David Miller wrote:
>
> I personally don't think it's unreasonable for GIT to have it's
> own customized allocator at least for certain object types.
Well, we actually already *do* have a customized allocator, but currently
only for the actual core "object descriptor" that really just has the SHA1
and object flags in it (and a few extra words depending on object type).
Those are critical for certain loads, and small too (so using the standard
allocator wasted a _lot_ of memory). In addition, they're fixed-size and
never free'd, so a specialized allocator really can do a lot better than
any general-purpose memory allocator ever could.
But the actual object *contents* are currently all allocated with whatever
the standard libc malloc/free allocator is that you compile for (or load
dynamically). Havign a specialized allocator for them is a much more
involved issue, exactly because we do have interesting allocation patterns
etc.
That said, at least those object allocations are all single-threaded (for
right now, at least), so even when git does multi-threaded stuff, the core
sha1_file.c stuff is always run under a single lock, and a simpler
allocator that doesn't care about threads is likely to be much better than
one that tries to have thread-local heaps etc.
I suspect that is what the google allocator does. It probably doesn't have
per-thread heaps, it just uses locking (and quite possibly things like
per-*size* heaps, which is much more memory-efficient and helps avoid some
of the fragmentation problems).
Locking is much slower than per-thread accesses, but it doesn't have the
issues with per-thread-fragmentation and all the problems with one thread
allocating and another one freeing.
Linus
^ permalink raw reply
* Re: Something is broken in repack
From: Jon Smirl @ 2007-12-12 17:12 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Nicolas Pitre, Junio C Hamano, gcc, Git Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712120826440.25032@woody.linux-foundation.org>
On 12/12/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>
> On Wed, 12 Dec 2007, Nicolas Pitre wrote:
> >
> > So... my conclusion is that the glibc allocator has fragmentation issues
> > with this work load, given the notable difference with the Google
> > allocator, which itself might not be completely immune to fragmentation
> > issues of its own.
>
> Yes.
>
> Note that delta following involves patterns something like
>
> allocate (small) space for delta
> for i in (1..depth) {
> allocate large space for base
> allocate large space for result
> .. apply delta ..
> free large space for base
> free small space for delta
> }
Is it hard to hack up something that statically allocates a big block
of memory per thread for these two and then just reuses it?
allocate (small) space for delta
allocate large space for base
The alternating between long term and short term allocations
definitely aggravates fragmentation.
>
> so if you have some stupid heap algorithm that doesn't try to merge and
> re-use free'd spaces very aggressively (because that takes CPU time!), you
> might have memory usage be horribly inflated by the heap having all those
> holes for all the objects that got free'd in the chain that don't get
> aggressively re-used.
>
> Threaded memory allocators then make this worse by probably using totally
> different heaps for different threads (in order to avoid locking), so they
> will *all* have the fragmentation issue.
>
> And if you *really* want to cause trouble for a memory allocator, what you
> should try to do is to allocate the memory in one thread, and free it in
> another, and then things can really explode (the freeing thread notices
> that the allocation is not in its thread-local heap, so instead of really
> freeing it, it puts it on a separate list of areas to be freed later by
> the original thread when it needs memory - or worse, it adds it to the
> local thread list, and makes it effectively totally impossible to then
> ever merge different free'd allocations ever again because the freed
> things will be on different heap lists!).
>
> I'm not saying that particular case happens in git, I'm just saying that
> it's not unheard of. And with the delta cache and the object lookup, it's
> not at _all_ impossible that we hit the "allocate in one thread, free in
> another" case!
>
> Linus
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [PATCH] builtin-clone: Implement git clone as a builtin command.
From: Daniel Barkalow @ 2007-12-12 18:00 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: Junio C Hamano, git
In-Reply-To: <1197473063.9269.20.camel@hinata.boston.redhat.com>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=utf-8, Size: 1769 bytes --]
On Wed, 12 Dec 2007, Kristian Høgsberg wrote:
> However, let me just say that the patch I sent is almost just that.
> Part of the patch refactors init-db to be useful from clone, part of the
> code is option parsing and figuring out the git dir, work tree. Also,
> the part of the patch that does 'git checkout' is approximately 20 lines
> that end up calling unpack_tre() and then write_cache(). The bulk of
> the work here is really just builtin boilerplate code, option parsing
> and the builtin-clone tasks you describe below (HEAD discovery, --shared
> and --reference optimizations and the local hardlink optimization - all
> these are in the 500 line builtin-clone.c I sent).
>
> And maybe it makes sense to use builtin-remote for the remote add -f
> part, but the fetch part of the patch is 10 lines to set up for
> fetch_pack(). So while I do agree that it makes sense to keep remotes
> handling in one place, doing the fetch_pack() in builtin-clone.c doesn't
> seem like a big duplication of code. And either way, I agree with
> Dscho, once we have either builtin-clone or builtin-fetch it's easier to
> share code and refactor, and there is not a strong reason to do one or
> the other first.
Er, we have builtin-fetch. We just don't have a way of calling it with all
of the option parsing done, but that should be easy. I was expecting that
step to get done when clone got converted, or maybe remote...
I agree that the checkout special case when the code knows in advance that
you don't have anything checked out beforehand is particularly trivial,
and it's probably just as easy to call unpack_trees() and write_cache() as
to use an actual checkout implementation.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH] Don't cache DESTDIR in perl/perl.mak.
From: Eric Wong @ 2007-12-12 18:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Gerrit Pape, git
In-Reply-To: <7vodcyl5dy.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> Hmph. That's reverting this:
>
> commit 4c5cf8c44ce06a79da5bafd4a92e6d6f598cea2e
> Author: Eric Wong <normalperson@yhbt.net>
> Date: Sun Aug 13 04:13:25 2006 -0700
>
> pass DESTDIR to the generated perl/Makefile
>
> Makes life for binary packagers easier, as the Perl modules will
> be installed inside DESTDIR.
>
> Signed-off-by: Eric Wong <normalperson@yhbt.net>
> Signed-off-by: Junio C Hamano <junkio@cox.net>
>
> Eric, care to comment?
I used to make a statically linked binary package for working on an
ancient box that didn't have a lot of libraries I wanted, and I probably
just called `make install' into DESTDIR as a single step without calling
`make' alone without DESTDIR argument, or I had DESTDIR set in
config.mak
--
Eric Wong
^ permalink raw reply
* Re: [PATCH 0/2] [RFT] git-svn: more efficient revision -> commit mapping
From: Eric Wong @ 2007-12-12 18:05 UTC (permalink / raw)
To: Sam Vilain; +Cc: Junio C Hamano, git, Harvey Harrison
In-Reply-To: <1197248646.7185.25.camel@brick>
Harvey Harrison <harvey.harrison@gmail.com> wrote:
> On Sun, 2007-12-09 at 12:56 -0800, Harvey Harrison wrote:
> > On Sat, 2007-12-08 at 23:27 -0800, Eric Wong wrote:
> > > This is very lightly tested, but describes the format I described in:
> > >
> > > http://article.gmane.org/gmane.comp.version-control.git/67126
> > >
> > > (more in the commit messages)
> > >
> > > I'll be out of town the next few days and I'm not sure how much I'll be
> > > able to follow up on it while I'm gone. Please test, especially if
> > > you're dealing with a repository where large .rev_db files are a
> > > problem.
> > >
> > > Junio: not intended for master just yet, but if you hear nothing but
> > > good things about it, feel free :)
> > Preliminary tests against the gcc repo are going swimmingly.
> >
> > Successful git svn rebase against trunk, doing a full git svn fetch
> > now to build rev_maps for all svn branches/tags. At halfway through
> > space has decreased from ~2GB to 17MB for about half of the needed
> > metadata.
> >
>
> Eric,
>
> I'm very happy with these patches. For the gcc repo, git-svn metadata
> has gone from over 5GB to 33MB. git-svn fetch/rebase are working fine,
> will shout if I see any odd behavior.
Harvey:
Thanks for the feedback. Glad it helps with gcc. I'll make unlinking
the index files the default tonight since it shouldn't hurt performance
enough to matter, and the disk savings is enough to justify it..
Sam (or anybody else using useSvmProps:
Do you have any feedback with svmProps enabled?
--
Eric Wong
^ permalink raw reply
* Re: [ANNOUNCE] ugit: a pyqt-based git gui // was: Re: If you would write git from scratch now, what would you change?
From: Johannes Schindelin @ 2007-12-12 18:15 UTC (permalink / raw)
To: Jason Sewall; +Cc: Shawn O. Pearce, David, Marco Costalba, Andy Parkins, git
In-Reply-To: <31e9dd080712120702k36a959cfh3e2a5c5fb076d922@mail.gmail.com>
Hi,
On Wed, 12 Dec 2007, Jason Sewall wrote:
> On Dec 12, 2007 12:23 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> > > I use git-gui and gitk for my git graphical needs because they rock
> > > and at the end of the day, the fonts and antialiasing aren't that
> > > big of a deal, especially since I'm usually doing quick scans and
> > > searches over the information those tools display, not reading
> > > novels in them.
> >
> > Good points. Features win over pretty most of the time. But at some
> > point pretty is important; especially to new user adoption. Plus if
> > you are looking at it all day long it shouldn't be jarring to the
> > eyes. But git-gui still isn't even where I want it ot be
> > feature-wise. E.g. I'd *love* to teach it inotify support, so you
> > don't even need to have that Rescan button.
>
> On that note, did you see what I wrote above, about having "split hunk"
> functionality?
I had a patch for splitting hunks in git-gui in August, but there were
some issues that I did not yet resolve. If you want to work on it, I'll
gladly share that patch with you.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] builtin-clone: Implement git clone as a builtin command.
From: Johannes Schindelin @ 2007-12-12 18:24 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: Junio C Hamano, Daniel Barkalow, git
In-Reply-To: <1197471866.9269.2.camel@hinata.boston.redhat.com>
Hi,
On Wed, 12 Dec 2007, Kristian H?gsberg wrote:
> On Wed, 2007-12-12 at 11:12 +0000, Johannes Schindelin wrote:
>
> > On Tue, 11 Dec 2007, Junio C Hamano wrote:
> ...
> > > * --shared optimization
> > >
> > > This is a very easy addition to "git remote add". You make sure
> > > that the added remote repository is on a local machine, and set
> > > up alternates to point at its object store.
> >
> > Concur.
> >
> > Since I want to lose that dependency on cpio on Windows (which we fake
> > by using tar), I'll implement this in C anyway.
>
> It's not used for --shared (which is just writing an alternates file),
> it's used for -l, hardlinking locally cloned repos. The code to replace
> cpio is already in the patch I sent, look for clone_local().
Sorry, that comment should have gone after another part of the original
message.
My only two excuses are that I am ill, and am overloaded with work.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 2/2] git-svn: get color config from --get-colorbool
From: Eric Wong @ 2007-12-12 18:27 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20071211062842.GB21768@coredump.intra.peff.net>
Jeff King <peff@peff.net> wrote:
> git-config recently learned a --get-colorbool option. By
> using it, we will get the same color=auto behavior that
> other git commands have.
>
> Specifically, this fixes the case where "color.diff = true"
> meant "always" in git-svn, but "auto" in other programs.
>
> Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Eric Wong <normalperson@yhbt.net>
> ---
> git-svn.perl | 35 ++---------------------------------
> 1 files changed, 2 insertions(+), 33 deletions(-)
>
> diff --git a/git-svn.perl b/git-svn.perl
> index 9f884eb..1c42c55 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -3969,39 +3969,7 @@ sub cmt_showable {
> }
>
> sub log_use_color {
> - return 1 if $color;
> - my ($dc, $dcvar);
> - $dcvar = 'color.diff';
> - $dc = `git-config --get $dcvar`;
> - if ($dc eq '') {
> - # nothing at all; fallback to "diff.color"
> - $dcvar = 'diff.color';
> - $dc = `git-config --get $dcvar`;
> - }
> - chomp($dc);
> - if ($dc eq 'auto') {
> - my $pc;
> - $pc = `git-config --get color.pager`;
> - if ($pc eq '') {
> - # does not have it -- fallback to pager.color
> - $pc = `git-config --bool --get pager.color`;
> - }
> - else {
> - $pc = `git-config --bool --get color.pager`;
> - if ($?) {
> - $pc = 'false';
> - }
> - }
> - chomp($pc);
> - if (-t *STDOUT || (defined $pager && $pc eq 'true')) {
> - return ($ENV{TERM} && $ENV{TERM} ne 'dumb');
> - }
> - return 0;
> - }
> - return 0 if $dc eq 'never';
> - return 1 if $dc eq 'always';
> - chomp($dc = `git-config --bool --get $dcvar`);
> - return ($dc eq 'true');
> + return $color || Git->repository->get_colorbool('color.diff');
> }
>
> sub git_svn_log_cmd {
> @@ -4060,6 +4028,7 @@ sub config_pager {
> } elsif (length $pager == 0 || $pager eq 'cat') {
> $pager = undef;
> }
> + $ENV{GIT_PAGER_IN_USE} = defined($pager);
> }
>
> sub run_pager {
> --
> 1.5.3.7.2230.g796d07-dirty
>
--
Eric Wong
^ permalink raw reply
* Re: [PATCH] builtin-clone: Implement git clone as a builtin command.
From: Kristian Høgsberg @ 2007-12-12 18:25 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0712121237540.5349@iabervon.org>
On Wed, 2007-12-12 at 13:00 -0500, Daniel Barkalow wrote:
> On Wed, 12 Dec 2007, Kristian Hgsberg wrote:
>
> > However, let me just say that the patch I sent is almost just that.
> > Part of the patch refactors init-db to be useful from clone, part of the
> > code is option parsing and figuring out the git dir, work tree. Also,
> > the part of the patch that does 'git checkout' is approximately 20 lines
> > that end up calling unpack_tre() and then write_cache(). The bulk of
> > the work here is really just builtin boilerplate code, option parsing
> > and the builtin-clone tasks you describe below (HEAD discovery, --shared
> > and --reference optimizations and the local hardlink optimization - all
> > these are in the 500 line builtin-clone.c I sent).
> >
> > And maybe it makes sense to use builtin-remote for the remote add -f
> > part, but the fetch part of the patch is 10 lines to set up for
> > fetch_pack(). So while I do agree that it makes sense to keep remotes
> > handling in one place, doing the fetch_pack() in builtin-clone.c doesn't
> > seem like a big duplication of code. And either way, I agree with
> > Dscho, once we have either builtin-clone or builtin-fetch it's easier to
> > share code and refactor, and there is not a strong reason to do one or
> > the other first.
>
> Er, we have builtin-fetch. We just don't have a way of calling it with all
> of the option parsing done, but that should be easy. I was expecting that
> step to get done when clone got converted, or maybe remote...
Ugh, I meant builtin-remote there, sorry. I use fetch_pack() like the shell
script does, and it seem a lot easier that trying to call fetch:
struct fetch_pack_args args;
args.uploadpack = option_upload_pack;
args.quiet = option_quiet;
args.fetch_all = 1;
args.lock_pack = 0;
args.keep_pack = 1;
args.depth = option_depth;
args.no_progress = 1;
refs = fetch_pack(&args, argv[0], 0, NULL, NULL);
Kristian
^ permalink raw reply
* Re: [PATCH] builtin-clone: Implement git clone as a builtin command.
From: Daniel Barkalow @ 2007-12-12 18:40 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: Junio C Hamano, git
In-Reply-To: <1197483943.10132.4.camel@hinata.boston.redhat.com>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=X-UNKNOWN, Size: 767 bytes --]
On Wed, 12 Dec 2007, Kristian Høgsberg wrote:
> Ugh, I meant builtin-remote there, sorry. I use fetch_pack() like the shell
> script does, and it seem a lot easier that trying to call fetch:
>
> struct fetch_pack_args args;
>
> args.uploadpack = option_upload_pack;
> args.quiet = option_quiet;
> args.fetch_all = 1;
> args.lock_pack = 0;
> args.keep_pack = 1;
> args.depth = option_depth;
> args.no_progress = 1;
>
> refs = fetch_pack(&args, argv[0], 0, NULL, NULL);
Ah, but that only works for git native protocol remote repositories.
Calling fetch instead would mean that other protocols also work without
any fuss.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: v1.5.4 plans
From: Eric Wong @ 2007-12-12 18:40 UTC (permalink / raw)
To: Junio C Hamano, David D. Kilzer; +Cc: git
In-Reply-To: <7vmysijhwq.fsf_-_@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> People might have noticed that I've been ignoring most of the new
> topics/enhancements for the past few days. Here is what I want to see
> happen until we declare v1.5.4.
>
> First, stabilize 'master' enough and tag v1.5.4-rc0 soon.
> * Eric's sanely-compact mapping from SVN rev-ids to git commits saw a
> positive feedback. I haven't carefully read that patch but it seemed
> sane and I'd like to have it in v1.5.4.
It's looking good for most users, I think it's safe enough for 1.5.4
> * I've seen t9119-git-svn-info.sh fail in my k.org private repository
> and have been skipping the test, but this needs to be diagnosed and
> fixed [*1*]. It could be just that the code is fine and the test is
> not rejecting SVN that is too-old. I dunno.
I wouldn't mind dropping this test for now.
100% output compatibility with SVN is too difficult to achieve
and IMHO not worth it for commands like `info' and `log'.
David:
I also noticed some race-conditions on this test when running this on my
Centrino laptop (my fastest box, but I rarely use it for git
development) and having git on my USB thumb drive. I'm pretty sure
these were caused by inconsistencies in handling timestamps on symlinks
vs timestamps on the files they link to.
> *1* t9119 first fails at the 6th test. Perhaps the test needs to check
> svn version first and stop testing this feature. This test does not
> fail on my personal box that has svn 1.4.2.
>
> * expecting success:
> (cd svnwc; svn info file) > expected.info-file &&
> (cd gitwc; git-svn info file) > actual.info-file &&
> git-diff expected.info-file actual.info-file
>
> diff --git a/expected.info-file b/actual.info-file
> index b1d57f4..997c927 100644
> --- a/expected.info-file
> +++ b/actual.info-file
> @@ -10,6 +10,5 @@ Last Changed Author: junio
> Last Changed Rev: 1
> Last Changed Date: 2007-12-10 22:18:12 +0000 (Mon, 10 Dec 2007)
> Text Last Updated: 2007-12-10 22:18:13 +0000 (Mon, 10 Dec 2007)
> -Properties Last Updated: 2007-12-10 22:18:13 +0000 (Mon, 10 Dec 2007)
> Checksum: 5bbf5a52328e7439ae6e719dfe712200
>
> * FAIL 6: info file
>
> (cd svnwc; svn info file) > expected.info-file &&
> (cd gitwc; git-svn info file) > actual.info-file &&
> git-diff expected.info-file actual.info-file
>
> : hera t/master; svn --version
> svn, version 1.3.2 (r19776)
--
Eric Wong
^ permalink raw reply
* Re: [PATCH] clone: support cloning full bundles
From: Johannes Schindelin @ 2007-12-12 18:46 UTC (permalink / raw)
To: Santi Béjar; +Cc: Git Mailing List
In-Reply-To: <8aa486160712120721k26158972qf11c889da98572c6@mail.gmail.com>
Hi,
On Wed, 12 Dec 2007, Santi B?jar wrote:
> On Dec 12, 2007 3:50 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> > On Wed, 12 Dec 2007, Santi B?jar wrote:
> >
> > > It still fails for incremental bundles.
> >
> > Of course it does. The whole point of incremental bundles is that
> > they do _not_ contain all objects, but rely on some objects being
> > present on the "fetch" side.
>
> I know this. But then there is no bundle equivalent of the shallow
> clones, as with:
>
> git clone --depth <depth> <repo>
Ah, thanks. Completely forgot about this.
Ciao,
Dscho
^ permalink raw reply
* Re: Something is broken in repack. Why not with fork and pipes?
From: J.C. Pizarro @ 2007-12-12 18:47 UTC (permalink / raw)
To: Linus Torvalds, Andreas Ericsson
Cc: David Miller, Nicolas Pitre, jonsmirl, Junio C Hamano, gcc, git
At http://gcc.gnu.org/ml/gcc/2007-12/msg00360.html, Andreas Ericsson
<ae@op5.se> wrote:
> If it's still an issue next week, we'll have a 16 core (8 dual-core cpu's)
> machine with some 32gb of ram in that'll be free for about two days.
> You'll have to remind me about it though, as I've got a lot on my mind
> these days.
>
>
> --
> Andreas Ericsson andreas.ericsson@op5.se
> OP5 AB www.op5.se
> Tel: +46 8-230225 Fax: +46 8-230231
It's good idea if it's for 24/365.25 that it does
autorepack-compute-again-again-again-those-unexplored-deltas of
git repositories in realtime. :D
Some body can do "git clone" that it could give smaller that one hour ago :D
-----------------------------------------------------------------
To Linus, Why don't you forget the threaded implementation of your repo-pack?
To imagine a "buggy bloated threading implementation originated to try it to
work only in HyperThreading Intel CPUs and 8 cores x 8 threads/core
Niagara Sparcs"
IMHO, in multicored machine, multiprocessed implementation of repo-pack perfomes
better than multithreaded implementation, although i've not their results.
It has not issue, not problem, etc. with memory allocation of threads,
so monothreaded memory allocation is simple and fast!
You can see "Why not with fork and pipes like in linux?" at
http://gcc.gnu.org/ml/gcc/2007-12/msg00203.html
http://gcc.gnu.org/ml/gcc/2007-12/msg00209.html
For easy implementation, don't use threads due to complicated condition races
between threads of multithreaded processes.
To use only condition races between monothreaded processes with select/epoll
only in the parent process. It's due to the KISS principle works.
The children processes share almost readed-only memory due to COW
(Copy On Write), so, before forking, the parent must to have a large
plain data structures in C for children. The children use pipes to
realize a complex intercommunication that the parent updates the
results computated by the children almost of the time.
Another implementation is that the children can realize a locked
load-and-store to/from unique filesystem's database if big memory to
store data is a big problem.
Another implementation is to consider children processes as intensive-CPU
slaves and parent process as the master that manipulates the big database.
If you want to measure the performance between multiprocessed vs multithreaded
implementation of repo-pack then you have to remember that
For same data input size and same data output size, to get the
seconds of your wall-clock or watch-clock as a measure of the benchmark
of this repo-pack.
The numeric data posted to mailing list about the timings dependently of # of
threads are bad measured because they don't say how is small the result repo.
and don't say if the results are the same independently of # of threads.
For good measures, we need "to plot the curves", e.g. based in
( # of threads, elapsed time of wall-clock, data input size, data output size )
and we can observe the intersection between above curves.
J.C.Pizarro
^ permalink raw reply
* Re: [ANNOUNCE] ugit: a pyqt-based git gui // was: Re: If you would write git from scratch now, what would you change?
From: Jason Sewall @ 2007-12-12 18:50 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Shawn O. Pearce, David, Marco Costalba, Andy Parkins, git
In-Reply-To: <Pine.LNX.4.64.0712121814260.27959@racer.site>
On Dec 12, 2007 1:15 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Wed, 12 Dec 2007, Jason Sewall wrote:
> > On that note, did you see what I wrote above, about having "split hunk"
> > functionality?
>
> I had a patch for splitting hunks in git-gui in August, but there were
> some issues that I did not yet resolve. If you want to work on it, I'll
> gladly share that patch with you.
Sure, send it along. I'm not going to make any promises, but I could
probably find time poke around in there.
Jason
^ permalink raw reply
* Re: Invalid dates in git log
From: Junio C Hamano @ 2007-12-12 18:57 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jeff King, Eirik Bjørsnøs, git
In-Reply-To: <Pine.LNX.4.64.0712121457280.27959@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Wed, 12 Dec 2007, Junio C Hamano wrote:
>
>> Author: Len Brown <len.brown@intel.com>
>> AuthorDate: Fri Apr 5 00:07:45 2019 -0500
>> Commit: Len Brown <len.brown@intel.com>
>> CommitDate: Tue Jul 12 00:12:09 2005 -0400
>>
>> author Len Brown <len.brown@intel.com> 1554440865 -0500
>> committer Len Brown <len.brown@intel.com> 1121141529 -0400
>>
>> [...] It looks like quite a random timestamp, and committer timestamp
>> look reasonable, relative to the other commits around it.
>
> It is quite possible that Len Brown had a similar problem to what I
> experienced yesterday: my clock was set one hour and 22 years into the
> future, but I have no idea how that happened. My only guess is a
> half-succeeded ntpdate call, but somehow I doubt that.
But how would you explain the more reasonable committer date?
I am guessing that this particular commit was rebased (using rebase,
rebase -i, stgit, guilt or something else), and whatever tool that was
used had some thinko that broke the author timestamp (and zone).
Nah, exonerate guilt and rebase -i from the list. The commit is from
July 2005 which means there wasn't much usable tool for rebasing in the
core distribution. Interactive rebase did not exist back then, and
rebase was done with "git-commit-script -m", which took an existing
commit and used its metainformation when creating a commit. This was
almost totally different codepath from what we have now. Not even
format-patch nor applymbox existed.
StGIT 0.4 (initial) was Jul 10, 2005, so it is _possible_ (I do not know
about plausibility) that it had an early bug that caused this, but if we
really want to figure it out we need to ask Len what was used, and I
suspect the most likely answer is "are you crazy enough to expect me to
remember?".
I am personally more interested in the other one, which was much more
recent incident, though.
^ 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