* Re: Subject: [PATCH 2/3] Let git-add--interactive read colors from .gitconfig
From: Jeff King @ 2007-11-13 7:26 UTC (permalink / raw)
To: Dan Zwell
Cc: Junio C Hamano, Shawn O. Pearce, Wincent Colaiuta,
Git Mailing List, Jonathan del Strother, Johannes Schindelin,
Frank Lichtenheld
In-Reply-To: <47391211.5000606@zwell.net>
On Mon, Nov 12, 2007 at 08:55:13PM -0600, Dan Zwell wrote:
> Anyway, I preferred the regex version for readability, though I should have
> used the /x modifier--it would still take two lines, but it would not need
Your regex is wrong, because it doesn't anchor at the beginning and end
of the string (so /red/ will match "supercaliredfragilistic", which is
probably not what you want). So you probably want /^red$/, which is
equivalent to using 'eq' with 'red'. Or, as Junio noted, you are overall
trying to say "is element $word in this list"; the canonical perl way of
doing that is to make the list a hash for quick lookup.
> to attempt two matches. As for misconfigured color configurations, should we
> catch that? I wrote this with the intent that it should ignore invalid color
> names, but it would probably be more useful to print a warning.
Your patch doesn't just ignore; sometimes it accidentally matches
invalid input (the example above is obviously silly, but consider what
accidentally omitting the space in "blinkblack" would do).
-Peff
^ permalink raw reply
* Re: [PATCH] git-send-email: show all headers when sending mail
From: Junio C Hamano @ 2007-11-13 7:28 UTC (permalink / raw)
To: David D. Kilzer; +Cc: git
In-Reply-To: <1194883317-11161-1-git-send-email-ddkilzer@kilzer.net>
"David D. Kilzer" <ddkilzer@kilzer.net> writes:
> +replace_header () {
> + EXPECTED=expected-show-all-headers &&
> + ACTUAL=actual-show-all-headers &&
> + REPLACEMENT=`cat ${ACTUAL} | grep "^$1:"` &&
> + if [ ! -z "${REPLACEMENT}" ]; then \
> + cat ${EXPECTED} | sed -e "s/^$1: .*\$/${REPLACEMENT}/" > ${EXPECTED}.$$ && \
> + mv -f ${EXPECTED}.$$ ${EXPECTED}
> + fi
> +}
If the actual output did not have an asked-for field,
REPLACEMENT will be empty and the breakage will go unnoticed,
won't it?
It would probably be better to write it this way:
test_expect_success 'Show all headers' '
git send-email \
--dry-run \
--from="Example <from@example.com>" \
--to=to@example.com \
--cc=cc@example.com \
--bcc=bcc@example.com \
--in-reply-to="<unique-message-id@example.com>" \
--smtp-sever relay.example.com \
$patches |
sed -e "s/^\(Date:\).*/1 DATE-STRING/" \
-e "s/^\(Message-Id:\).*/1 ID-STRING/" \
-e "s/^\(X-Mailer:\).*/1 X-MAILER-STRING/" \
>actual &&
diff -u expected actual
'
and prepare the expected output with the varying field already
replaced with the placeholder string.
Oh, by the way, do not cat a single file and pipe it to another
command. There may still be a few such stupidity in our test
scripts but let's not add even more of them...
^ permalink raw reply
* Re: Subject: [PATCH 2/3] Let git-add--interactive read colors from .gitconfig
From: Junio C Hamano @ 2007-11-13 7:29 UTC (permalink / raw)
To: Dan Zwell
Cc: Jeff King, Shawn O. Pearce, Wincent Colaiuta, Git Mailing List,
Jonathan del Strother, Johannes Schindelin, Frank Lichtenheld
In-Reply-To: <47391211.5000606@zwell.net>
Dan Zwell <dzwell@gmail.com> writes:
> ... I wrote this with the intent
> that it should ignore invalid color names, but it would probably be
> more useful to print a warning.
But the point is, that you are not ignoring invalid color names
but instead giving back a random match aren't you?
^ permalink raw reply
* Re: git diff woes
From: Andreas Ericsson @ 2007-11-13 7:40 UTC (permalink / raw)
To: Miles Bader; +Cc: Junio C Hamano, Johannes Schindelin, Git Mailing List
In-Reply-To: <buomytin9dz.fsf@dhapc248.dev.necel.com>
Miles Bader wrote:
> Andreas Ericsson <ae@op5.se> writes:
>> I notice it, and I don't like it. I guess I'm just used to git being
>> smarter than their GNU tool equivalents, especially since it only ever
>> applies patches in full.
>
> It's not at all obvious that this behavior is actually wrong -- it seems
> perfectly reasonable to use either old or new text for the hunk headers.
>
Right, which is why I've made it configurable.
> It hardly matters really, since that particular output is just "useful
> noise" to provide a bit of helpful context for human readers, and humans
> (unlike programs) are notoriously good at not being bothered by such
> things. Er, well most humans anyway.
>
I wouldn't have reacted either, except that this time someone asked me to
review a branch early in the morning because he had introduced a bug in the
process, and the hunk header information made me assume the wrong hunk of
the patch was the culprit.
On the one hand, it wouldn't have been so much of a problem if the developer
in question would have followed my suggestion of committing small and making
sure the commit message describes everything that's done. On the other hand,
a tool fooling a human isn't a good thing either, even if said human is not
really in shape for using said tool.
Granted, the new form can still fool people, but for archeology excursions
I think it's definitely right to use the "new" funcname in the hunk header.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH] Update the tracking references only if they were succesfully updated on remote
From: Jeff King @ 2007-11-13 7:52 UTC (permalink / raw)
To: Alex Riesen; +Cc: git, Junio C Hamano
In-Reply-To: <20071112213938.GC2918@steel.home>
On Mon, Nov 12, 2007 at 10:39:38PM +0100, Alex Riesen wrote:
> It fixes the bug where local tracing branches were filled with zeroed SHA-1
> if the remote branch was not updated because, for instance, it was not
> an ancestor of the local (i.e. had other changes).
>
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> ---
>
> Jeff, I think your change (334f4831e5a77) was either not complete or
> got broken some time later.
Yes, some of the error information placed in 'ret' was getting lost.
Daniel and I discussed some fixes, but haven't done a final patch yet.
Your patch doesn't work because the assumption that
is_null_sha1(refs->new_sha1) signals error is not correct. This is also
the case for deleted refs, which means that we are failing to update
tracking branches for successfully deleted refs.
I'd like to have a patch that accurately tracks per-ref errors,
including ones from the remote. That not only will give us more accurate
status reporting, but fixes like this will be much easier. Let me see if
I can put something together.
-Peff
^ permalink raw reply
* Re: [PATCH 4/2] Fix parent rewriting in --early-output
From: Sven Verdoolaege @ 2007-11-13 7:53 UTC (permalink / raw)
To: Linus Torvalds
Cc: Junio C Hamano, Paul Mackerras, Marco Costalba, Git Mailing List
In-Reply-To: <alpine.LFD.0.9999.0711122309270.2786@woody.linux-foundation.org>
On Mon, Nov 12, 2007 at 11:16:08PM -0800, Linus Torvalds wrote:
> On Mon, 12 Nov 2007, Linus Torvalds wrote:
> >
> > I think the real problem is that "TREECHANGE" has the wrong polarity. It
> > should default to always being set, and then we could actively clear it
> > when we do the work to say "it's the same tree". Instead, we default it to
> > being the same (which triggers parent rewriting), and only later may we
> > notice that it wasn't the same.
>
> So, maybe the proper solution is to say "commits are assumed to be
> different to their parents, but we have an explicit bit saying TREESAME
> when we find them to be the same".
FWIW, I like it.
I had basically the same patch in my git-rewrite-commits series
(except that I called it "PRUNED" instead of "TREESAME"), but I
don't think I even sent it to the list, because I was told there
was no use.
skimo
^ permalink raw reply
* Re: [PATCH] Beautify the output of send-pack a bit
From: Jeff King @ 2007-11-13 7:55 UTC (permalink / raw)
To: Alex Riesen; +Cc: git, Junio C Hamano
In-Reply-To: <20071112221140.GD2918@steel.home>
On Mon, Nov 12, 2007 at 11:11:40PM +0100, Alex Riesen wrote:
> Cluster the errors regarding ancestry violation and output them
> in one batch, together with a hint to pull before pushing.
I think this is a good direction, but I think it might be simpler if
just record error information for each ref as we go, and then dump all
of them at the end. That will let us add remote-generated error
information easily.
-Peff
^ permalink raw reply
* Re: [PATCH 4/2] Fix parent rewriting in --early-output
From: Shawn O. Pearce @ 2007-11-13 8:01 UTC (permalink / raw)
To: Junio C Hamano
Cc: Linus Torvalds, Paul Mackerras, Marco Costalba, Git Mailing List
In-Reply-To: <7v1wauzomr.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> I have to wonder what would happen if a much higher level caller
> caused the objects to get parsed before coming into the revision
> walking machinery, e.g. after the command line processing for
> A...B walked the ancestry chain until their common ancestors are
> found. So these commits between A and B are parsed, but the
> revision limiting machinery hasn't done its operation to set
> TREECHANGE and/or UNINTERESTING in add_parents_to_list() on
> these commits yet.
That's one of the problems with the way the revision walking
machinery is built. Its fast, but it can really only be used once.
My series about making the allocators able to free their nodes was
to allow resetting the entire machinary for another user, but as you
pointed out how do we decide when we can do a reset and invalidate
all prior struct commit*?
--
Shawn.
^ permalink raw reply
* [PATCH] Handle broken vsnprintf implementations in strbuf
From: Shawn O. Pearce @ 2007-11-13 8:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Solaris 9's vsnprintf implementation returns -1 if we pass it a
buffer of length 0. The only way to get it to give us the actual
length necessary for the formatted string is to grow the buffer
out to have at least 1 byte available in the strbuf and then ask
it to compute the length.
If the available space is 0 I'm growing it out by 64 to ensure
we will get an accurate length estimate from all implementations.
Some callers may need to grow the strbuf again but 64 should be a
reasonable enough initial growth.
We also no longer silently fail to append to the string when we are
faced with a broken vsnprintf implementation. On Solaris 9 this
silent failure caused me to no longer be able to execute "git clone"
as we tried to exec the empty string rather than "git-clone".
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
strbuf.c | 7 ++++---
trace.c | 4 ++--
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index dbd8c4b..b9b194b 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -118,12 +118,13 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
int len;
va_list ap;
+ if (!strbuf_avail(sb))
+ strbuf_grow(sb, 64);
va_start(ap, fmt);
len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
va_end(ap);
- if (len < 0) {
- len = 0;
- }
+ if (len < 0)
+ die("your vsnprintf is broken");
if (len > strbuf_avail(sb)) {
strbuf_grow(sb, len);
va_start(ap, fmt);
diff --git a/trace.c b/trace.c
index 69fa05e..0d89dbe 100644
--- a/trace.c
+++ b/trace.c
@@ -72,7 +72,7 @@ void trace_printf(const char *fmt, ...)
if (!fd)
return;
- strbuf_init(&buf, 0);
+ strbuf_init(&buf, 64);
va_start(ap, fmt);
len = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap);
va_end(ap);
@@ -103,7 +103,7 @@ void trace_argv_printf(const char **argv, int count, const char *fmt, ...)
if (!fd)
return;
- strbuf_init(&buf, 0);
+ strbuf_init(&buf, 64);
va_start(ap, fmt);
len = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap);
va_end(ap);
--
1.5.3.5.1661.gcbf0
^ permalink raw reply related
* Re: [PATCH 4/2] Fix parent rewriting in --early-output
From: Junio C Hamano @ 2007-11-13 8:24 UTC (permalink / raw)
To: Shawn O. Pearce
Cc: Linus Torvalds, Paul Mackerras, Marco Costalba, Git Mailing List
In-Reply-To: <20071113080125.GB14735@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Junio C Hamano <gitster@pobox.com> wrote:
>
>> I have to wonder what would happen if a much higher level caller
>> caused the objects to get parsed before coming into the revision
>> walking machinery, e.g. after the command line processing for
>> A...B walked the ancestry chain until their common ancestors are
>> found. So these commits between A and B are parsed, but the
>> revision limiting machinery hasn't done its operation to set
>> TREECHANGE and/or UNINTERESTING in add_parents_to_list() on
>> these commits yet.
>
> That's one of the problems with the way the revision walking
> machinery is built. Its fast, but it can really only be used once.
Yes but not quite. As long as you do not use overlapping set of
flag bits without cleaning, you are almost Ok.
The reason I say "almost" is that I think the true problem with
the first patch by Linus was to load "parsed" bit any semantics
other than "we have read and parsed the data so do not bother
rereading it". If it used another mechanism (e.g. another flag
bit that means "we have done TREECHANGE and stuff"), returning
rewrite_one_ok to punt when seeing an unprocessed node would
have been safe, as it would not have munged the parent list.
The replacement "TREESAME" patch would work much better without
using such an extra bit, because it allows us to directly check
"if we already decided this does not change from the parent",
and the lack of the bit covers both "we haven't processed it"
and "we processed but we do not want to prune" cases, and not
pruning is safe and easily and correctly re-processible in the
post clean-up phase of the early_output series.
But in general, you're right. An operation that changes the
ancestry shape is irreversible, and you would need to cause
reparsing of the commit objects after you are done with revision
traversal, and at that point, discarding and re-reading
everything from scratch, although is heavy-handed, is one
plausible approach to tackle the problem.
^ permalink raw reply
* Re: Subject: [PATCH 2/3] Let git-add--interactive read colors from .gitconfig
From: Dan Zwell @ 2007-11-13 8:25 UTC (permalink / raw)
To: Junio C Hamano
Cc: Dan Zwell, Jeff King, Shawn O. Pearce, Wincent Colaiuta,
Git Mailing List, Jonathan del Strother, Johannes Schindelin,
Frank Lichtenheld
In-Reply-To: <7v4pfqwqln.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> But the point is, that you are not ignoring invalid color names
> but instead giving back a random match aren't you?
No, if there's no match, the token is ignored. False matches are
possible in some cases (the bogus config option "colored" would match
"red", for example), so I will follow your suggestion with the hash,
after all. I'll send out the next revised patches in a day or two--I've
made most of the changes you and Jeff suggested, but I need to double check.
Dan
^ permalink raw reply
* Re: wishlist: git info
From: Thomas Neumann @ 2007-11-13 8:27 UTC (permalink / raw)
To: git
In-Reply-To: <fhaol0$p5r$1@ger.gmane.org>
> Perhaps also project description (if it exists?)
one can specify a project description? I did not even know this. But
yes, this would be useful, too.
In general I think git info should show everything to quickly understand
what is currently checked out. The name of the current branch should
probably be included, too.
I use an alias with the commands proposed by Alex Riessen for now, but a
more general command would be nice.
Thomas
^ permalink raw reply
* Re: [PATCH 4/2] Fix parent rewriting in --early-output
From: Junio C Hamano @ 2007-11-13 8:48 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Paul Mackerras, Marco Costalba, Git Mailing List
In-Reply-To: <alpine.LFD.0.9999.0711122309270.2786@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> This solves the problem quite naturally, because any tree that hasn't been
> parsed - or even if it *has* been parsed, but just hasn't gone through
> the compare function - will then always be seen as "different" and thus
> interesting.
>
> This fairly straight-forward patch seems to work. It *replaces* the
> pervious "patch 4/2", and yes, Junio, I think you were very right to
> complain about that one.
>
> How does this one feel?
I think this is very natural and I like it.
I did not complain but just said I was puzzled, but after
thinking about this a bit I probably should have ;-).
^ permalink raw reply
* Re: [BUG] fast-import producing very deep tree deltas
From: Shawn O. Pearce @ 2007-11-13 8:53 UTC (permalink / raw)
To: Brian Downing; +Cc: git
In-Reply-To: <20071112110354.GP6212@lavos.net>
Brian Downing <bdowning@lavos.net> wrote:
> I've happened upon a case where fast-import produces deep tree deltas.
> How deep? Really deep. 6035 entries deep to be precise for this case:
>
> depths: count 135970 total 120567366 min 0 max 6035 mean 886.72 median 3 std_dev 1653.48
>
> 27b8a20bdf39fecd917e8401d3499013e49449d0 tree 32 99609547 6035 0000000000000000000000000000000000000000
>
> This was with git-fast-import from 'next' as of a couple days ago,
> run with the default options (no --depth passed in).
>
> Needless to say the pack that resulted was just about useless. Trying to
> repack it resulted in the "counting objects" phase running at about five
> objects per second.
Heh.
I think what's happening here is your active branch cache isn't
big enough. We're swapping out the branch and thus recycling the
tree information (struct tree_content) back into the free pool.
When we later reload the tree we set the delta_depth to 0 but we
kept the tree we just reloaded as a delta base.
So if the tree we reloaded was already at the maximum we wouldn't
know it and make the new tree a delta. Multiply the number of times
the branch cache has to swap out the tree times max_depth (10) and
you get the maximum delta depth of a tree created by fast-import.
Given your above data of 6035 I'm guessing your active branch cache
had to swap the branch out 603/604 times during this import.
I think the fix is going to involve caching the depth within struct
object_entry so we can restore it when the tree is reloaded.
--
Shawn.
^ permalink raw reply
* [PATCH] diffcore: Allow users to decide what funcname to use
From: Andreas Ericsson @ 2007-11-13 9:15 UTC (permalink / raw)
To: Miles Bader; +Cc: Junio C Hamano, Johannes Schindelin, Git Mailing List
In-Reply-To: <473954F8.8070908@op5.se>
Andreas Ericsson wrote:
> Miles Bader wrote:
>> Andreas Ericsson <ae@op5.se> writes:
>>> I notice it, and I don't like it. I guess I'm just used to git being
>>> smarter than their GNU tool equivalents, especially since it only ever
>>> applies patches in full.
>>
>> It's not at all obvious that this behavior is actually wrong -- it seems
>> perfectly reasonable to use either old or new text for the hunk headers.
>>
>
> Right, which is why I've made it configurable.
>
My git hacking has been stalled at the office for now, and I'm swanked at
home since my girlfriend just moved in and brought temporary pandemonium
with her. Here's what I've got now. It passes all tests, but there are no
new ones added. Documentation also needs updating.
Extract with
sed -n -e /^#CUTSTART/,/^#CUTEND/p -e /^#/d
#CUTSTART---%<---%<---%<---
From: Andreas Ericsson <ae@op5.se>
Date: Tue, 13 Nov 2007 09:47:43 +0100
Subject: [PATCH] diffcore: Allow users to decide what funcname to use
The function name being printed with the header of each
hunk is fetched from the "old" file today. Since git by
default applies patches either in full or not at all it's
arguably more correct to use the function from the "new"
file, at least when manually reviewing commits.
I stumbled upon this hunk when reviewing a series of
commits which caused the resulting code to segfault
under certain circumstances. Several hunks before, the
function declaration was changed and "status" was now
declared as an auto variable of type "int". The hunk
looks obviously bogus, and since I wasn't properly
awake, I reported this hunk to be the bogus one.
@@ -583,75 +346,100 @@ double jitter_request(int *status){
context
context
if(!syncsource_found){
- *status = STATE_UNKNOWN;
+ status = STATE_WARNING;
if(verbose) printf("warning: no sync source found\n")
}
This is what GNU "diff -p" would have reported under the
same circumstances, but GNU diff has no notion of version
control, and as such will not know if it's being used on
content where the patch by definition will apply in full.
Git can be smarter than that, and imo it should. This
patch lets the diffcore grok a new configuration variable,
"diff.funcnames", which can be set to "new", "old", or a
boolean value, which will cause it to be "old" (for 'true')
and 'none' (for 'false').
Signed-off-by: Andreas Ericsson <ae@op5.se>
---
diff.c | 20 ++++++++++++++++++--
diffcore-break.c | 4 ++--
xdiff/xdiff.h | 3 ++-
xdiff/xemit.c | 7 ++++++-
4 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/diff.c b/diff.c
index 6bb902f..057bba8 100644
--- a/diff.c
+++ b/diff.c
@@ -20,6 +20,7 @@
static int diff_detect_rename_default;
static int diff_rename_limit_default = 100;
static int diff_use_color_default;
+static unsigned long xdl_emit_flags = XDL_EMIT_FUNCNAMES;
int diff_auto_refresh_index = 1;
static char diff_colors[][COLOR_MAXLEN] = {
@@ -178,6 +179,20 @@ int git_diff_ui_config(const char *var, const char *value)
color_parse(value, var, diff_colors[slot]);
return 0;
}
+ if (!prefixcmp(var, "diff.funcnames")) {
+ if (!value)
+ xdl_emit_flags = XDL_EMIT_COMMON;
+ else if (!strcasecmp(value, "new"))
+ xdl_emit_flags = XDL_EMIT_FUNCNAMES_NEW;
+ else if (!strcasecmp(value, "old") || !strcasecmp(value, "default"))
+ xdl_emit_flags = XDL_EMIT_FUNCNAMES;
+ else {
+ if (git_config_bool(var, value))
+ xdl_emit_flags = XDL_EMIT_FUNCNAMES;
+ else
+ xdl_emit_flags = XDL_EMIT_COMMON;
+ }
+ }
return git_default_config(var, value);
}
@@ -1332,7 +1347,8 @@ static void builtin_diff(const char *name_a,
ecbdata.found_changesp = &o->found_changes;
xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
xecfg.ctxlen = o->context;
- xecfg.flags = XDL_EMIT_FUNCNAMES;
+ xecfg.flags = xdl_emit_flags;
+
if (funcname_pattern)
xdiff_set_find_func(&xecfg, funcname_pattern);
if (!diffopts)
@@ -2844,7 +2860,7 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)
xpp.flags = XDF_NEED_MINIMAL;
xecfg.ctxlen = 3;
- xecfg.flags = XDL_EMIT_FUNCNAMES;
+ xecfg.flags = xdl_emit_flags;
ecb.outf = xdiff_outf;
ecb.priv = &data;
xdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
diff --git a/diffcore-break.c b/diffcore-break.c
index c71a226..048ec25 100644
--- a/diffcore-break.c
+++ b/diffcore-break.c
@@ -257,8 +257,8 @@ void diffcore_merge_broken(void)
if (!p)
/* we already merged this with its peer */
continue;
- else if (p->broken_pair &&
- !strcmp(p->one->path, p->two->path)) {
+
+ if (p->broken_pair && !strcmp(p->one->path, p->two->path)) {
/* If the peer also survived rename/copy, then
* we merge them back together.
*/
diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index c00ddaa..326e1df 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -41,7 +41,8 @@ extern "C" {
#define XDL_EMIT_FUNCNAMES (1 << 0)
#define XDL_EMIT_COMMON (1 << 1)
-
+#define XDL_EMIT_FUNCNAMES_NEW (1 << 2)
+
#define XDL_MMB_READONLY (1 << 0)
#define XDL_MMF_ATOMIC (1 << 0)
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index d3d9c84..51dd085 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -149,7 +149,12 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
* Emit current hunk header.
*/
- if (xecfg->flags & XDL_EMIT_FUNCNAMES) {
+ if (xecfg->flags & XDL_EMIT_FUNCNAMES_NEW) {
+ xdl_find_func(&xe->xdf2, s2, funcbuf,
+ sizeof(funcbuf), &funclen,
+ ff, xecfg->find_func_priv);
+ }
+ else if (xecfg->flags & XDL_EMIT_FUNCNAMES) {
xdl_find_func(&xe->xdf1, s1, funcbuf,
sizeof(funcbuf), &funclen,
ff, xecfg->find_func_priv);
--
1.5.3.5.1527.g6161
#CUTEND---%<---%<---%<---
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply related
* Re: [BUG] fast-import producing very deep tree deltas
From: Shawn O. Pearce @ 2007-11-13 9:27 UTC (permalink / raw)
To: Brian Downing; +Cc: git
In-Reply-To: <20071113085307.GC14735@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> wrote:
> Brian Downing <bdowning@lavos.net> wrote:
> > I've happened upon a case where fast-import produces deep tree deltas.
> > How deep? Really deep. 6035 entries deep to be precise for this case:
> >
> > depths: count 135970 total 120567366 min 0 max 6035 mean 886.72 median 3 std_dev 1653.48
> >
> > 27b8a20bdf39fecd917e8401d3499013e49449d0 tree 32 99609547 6035 0000000000000000000000000000000000000000
> >
> > This was with git-fast-import from 'next' as of a couple days ago,
> > run with the default options (no --depth passed in).
> >
> > Needless to say the pack that resulted was just about useless. Trying to
> > repack it resulted in the "counting objects" phase running at about five
> > objects per second.
Brian, does this fix it?
--8>--
From ff39dd457564b9198344e0cc785afa8cac05b486 Mon Sep 17 00:00:00 2001
From: Shawn O. Pearce <spearce@spearce.org>
Date: Tue, 13 Nov 2007 04:26:24 -0500
Subject: [PATCH] Don't allow fast-import tree delta chains to exceed maximum depth
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Brian Downing noticed fast-import can produce tree depths of up
to 6,035 objects and even deeper. Long delta chains can create
very small packfiles but cause problems during repacking as git
needs to unpack each tree to count the reachable blobs.
What's happening here is the active branch cache isn't big enough.
We're swapping out the branch and thus recycling the tree information
(struct tree_content) back into the free pool. When we later reload
the tree we set the delta_depth to 0 but we kept the tree we just
reloaded as a delta base.
So if the tree we reloaded was already at the maximum depth we
wouldn't know it and make the new tree a delta. Multiply the
number of times the branch cache has to swap out the tree times
max_depth (10) and you get the maximum delta depth of a tree created
by fast-import. In Brian's case above the active branch cache had
to swap the branch out 603/604 times during this import to produce
a tree with a delta depth of 6035.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
fast-import.c | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index f93d7d6..215f1e7 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -153,13 +153,16 @@ Format of STDIN stream:
#define PACK_ID_BITS 16
#define MAX_PACK_ID ((1<<PACK_ID_BITS)-1)
+#define DEPTH_BITS 13
+#define MAX_DEPTH ((1<<DEPTH_BITS)-1)
struct object_entry
{
struct object_entry *next;
uint32_t offset;
- unsigned type : TYPE_BITS;
- unsigned pack_id : PACK_ID_BITS;
+ uint32_t type : TYPE_BITS,
+ pack_id : PACK_ID_BITS,
+ depth : DEPTH_BITS;
unsigned char sha1[20];
};
@@ -1084,6 +1087,7 @@ static int store_object(
delta_count_by_type[type]++;
last->depth++;
+ e->depth = last->depth;
hdrlen = encode_header(OBJ_OFS_DELTA, deltalen, hdr);
write_or_die(pack_data->pack_fd, hdr, hdrlen);
@@ -1097,6 +1101,7 @@ static int store_object(
} else {
if (last)
last->depth = 0;
+ e->depth = 0;
hdrlen = encode_header(type, dat->len, hdr);
write_or_die(pack_data->pack_fd, hdr, hdrlen);
pack_size += hdrlen;
@@ -1160,7 +1165,7 @@ static void load_tree(struct tree_entry *root)
if (myoe && myoe->pack_id != MAX_PACK_ID) {
if (myoe->type != OBJ_TREE)
die("Not a tree: %s", sha1_to_hex(sha1));
- t->delta_depth = 0;
+ t->delta_depth = myoe->depth;
buf = gfi_unpack_entry(myoe, &size);
} else {
enum object_type type;
@@ -2289,8 +2294,11 @@ int main(int argc, const char **argv)
}
else if (!prefixcmp(a, "--max-pack-size="))
max_packsize = strtoumax(a + 16, NULL, 0) * 1024 * 1024;
- else if (!prefixcmp(a, "--depth="))
+ else if (!prefixcmp(a, "--depth=")) {
max_depth = strtoul(a + 8, NULL, 0);
+ if (max_depth > MAX_DEPTH)
+ die("--depth cannot exceed %u", MAX_DEPTH);
+ }
else if (!prefixcmp(a, "--active-branches="))
max_active_branches = strtoul(a + 18, NULL, 0);
else if (!prefixcmp(a, "--import-marks="))
--
1.5.3.5.1661.gcbf0
--
Shawn.
^ permalink raw reply related
* Re: wishlist: git info
From: Jakub Narebski @ 2007-11-13 9:44 UTC (permalink / raw)
To: git
In-Reply-To: <fhbn50$uqu$1@ger.gmane.org>
Thomas Neumann wrote:
>> Perhaps also project description (if it exists?)
> one can specify a project description? I did not even know this. But
> yes, this would be useful, too.
It is project description for gitweb (git web interface), and I think also
for other web interfaces (cgit, wit, git-php). Or rather *repository*
description. It is in .git/description (in 'description' file in git
repository).
Unfortunately by default it contains (see: ${tempate_dir}/description)
Unnamed repository; edit this file to name it for gitweb.
> In general I think git info should show everything to quickly understand
> what is currently checked out. The name of the current branch should
> probably be included, too.
Name of current branch, current directory, name of topdir (with '.git'
stripped if it is bare repository), perhaps oneline description of top
commit. And if branch is under StGit or Guilt (patch management interfaces)
control.
See also helper functions to set shell prompt in
contrib/completion/git-completion.bash.
I for example have "1760:[gitweb/web!git]$ " as bash prompt:
- 1760 is number of command in history,
- 'gitweb/web' is name of branch I am on,
- 'git' is the name of repository
This migitates need got 'git-info' command.
> I use an alias with the commands proposed by Alex Riessen for now, but a
> more general command would be nice.
You can always write[*1*] git-info.sh or git-info.perl[*2*], and install it
as git-info in your git installation. And send patches here, to git mailing
list, for comments.
Footnotes:
----------
[*1*] Even if one of the most common complaints is "too many user-visible
commands".
[*2*] Scripts are better for ptototyping. Choose your own poison: POSIX
shell or Perl...
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: Cloning empty repositories, was Re: What is the idea for bare repositories?
From: Matthieu Moy @ 2007-11-13 9:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Bill Lear, Jan Wielemaker, git
In-Reply-To: <7v4pfr2kmh.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> But both of Johannes's points apply equally well to an empty
> bare repository and to an empty non bare repository. IOW,
> bareness does not matter to the suggestion Johannes gave.
He was suggesting to create the initial commit before cloning:
>> So you need to populate the repository before starting _anyway_.
To create an initial commit in a non-bare repository, I put files in
it, git add, and git commit.
To create an initial commit in a bare repository, the most natural way
for me is to clone it, create the commit in the clone, and then push.
Bare-ness _does_ matter for that.
I repeat the use-case I mentionned above :
,----
| a typical use-case is when I want to create a new project. I'd
| like to initialize an empty bare repo on my backed up disk, and then
| clone it to my local-fast-unreliable disk to get a working copy and do
| the first commit there.
`----
I find this quite natural, and up to now, no one gave me either a
rationale not to do that, or a _simple_ way to achieve this. As I
said, it's currently not _very_ hard to do, but I have to edit
.git/config by hand, while git clone knows how to do this much faster
than I for non-empty repositories.
--
Matthieu
^ permalink raw reply
* Re: Subject: [PATCH 2/3] Let git-add--interactive read colors from .gitconfig
From: Jakub Narebski @ 2007-11-13 9:46 UTC (permalink / raw)
To: git
In-Reply-To: <47395F63.8040306@zwell.net>
Dan Zwell wrote:
> Junio C Hamano wrote:
>> But the point is, that you are not ignoring invalid color names
>> but instead giving back a random match aren't you?
>
> No, if there's no match, the token is ignored. False matches are
> possible in some cases (the bogus config option "colored" would match
> "red", for example),
Match /\b(?:red|...)\b/ then
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: Cloning from kernel.org, then switching to another repo
From: Matthieu Moy @ 2007-11-13 9:52 UTC (permalink / raw)
To: Jon Smirl; +Cc: Jeff King, Johannes Schindelin, Git Mailing List
In-Reply-To: <9e4733910711122030q7bbf6057ubb6b5b27e1885500@mail.gmail.com>
"Jon Smirl" <jonsmirl@gmail.com> writes:
> Execute bit was not set. I just set it for all the scripts. +x is not
> getting turned on with a default git init-db. I just made a new repo
> to check, no +x on the scripts.
That's by design: "git init" gives you _example_ hooks, but they won't
run until you activate them explicitely with the appropriate chmod.
That said, I'm not sure there's a really good reason not to run
update-server-info by default on push. It doesn't cost much and saves
a lot of troubles for beginners. Perhaps there are cases where the
performance cost is non-negligible.
--
Matthieu
^ permalink raw reply
* Re: [PATCH 4/2] Fix parent rewriting in --early-output
From: Paul Mackerras @ 2007-11-13 9:59 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Marco Costalba, Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.0.9999.0711122046570.2786@woody.linux-foundation.org>
Linus Torvalds writes:
> However, while the parent is now correctly rewritten, it looks like gitk
> is confused by this. Gitk will remember the original parent information,
> even if a replay has given new parenthood information. Since the partial
> early-output information is triggered by timing, this means that gitk will
> show some totally random parent that quite possibly won't even be part of
> the final commit set at all!
Yep. It will be a little complex to deal with that because there are
bits of state that I set up for the parents, and if they're the wrong
parents, I'll have to go back and undo that.
In fact it would be easier for me if, instead of getting the id of
some random ancestor commit, I got an explicit indication to say
"unknown parent", such as just a "-" in place of the id of the
unknown parent(s). Would that be doable? I could then just not do
the processing for any unknown parent, and make sure to do it when I
see the final version of the commit.
Also, I have just about worked out an efficient way to do the commit
reordering incrementally, which would let me not use --topo-order or
--date-order, and display commits as they come in. I'll have to see
whether that turns out to be better overall than using --early-output.
Paul.
^ permalink raw reply
* Re: Cloning empty repositories, was Re: What is the idea for bare repositories?
From: Shawn O. Pearce @ 2007-11-13 10:02 UTC (permalink / raw)
To: Matthieu Moy
Cc: Junio C Hamano, Johannes Schindelin, Bill Lear, Jan Wielemaker,
git
In-Reply-To: <vpqzlxiiii6.fsf@bauges.imag.fr>
Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> I repeat the use-case I mentionned above :
>
> ,----
> | a typical use-case is when I want to create a new project. I'd
> | like to initialize an empty bare repo on my backed up disk, and then
> | clone it to my local-fast-unreliable disk to get a working copy and do
> | the first commit there.
> `----
>
> I find this quite natural, and up to now, no one gave me either a
> rationale not to do that, or a _simple_ way to achieve this. As I
> said, it's currently not _very_ hard to do, but I have to edit
> .git/config by hand, while git clone knows how to do this much faster
> than I for non-empty repositories.
Its a goal to redefine git-clone as the following, as that is
really all it does:
mkdir foo && cd foo && git init &&
git remote add -f origin $url &&
git checkout -b master origin/master
So setting up an empty tree is basically that:
mkdir foo && cd foo && git init &&
git remote add origin $url
Is that really so difficult? git-clone is a handy crutch for when
we didn't have things like git-remote. Or remote tracking branches.
IMHO the above may seem a little low level but it may make it easier
to teach to newbies. They are more likely to grasp the concept of
their repository being just like someone else's, and that they can
track other repositories beyond just their origin.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] diffcore: Allow users to decide what funcname to use
From: Jakub Narebski @ 2007-11-13 10:03 UTC (permalink / raw)
To: git
In-Reply-To: <47396B4C.6070406@op5.se>
Andreas Ericsson wrote:
> Git can be smarter than that, and imo it should. This
> patch lets the diffcore grok a new configuration variable,
> "diff.funcnames", which can be set to "new", "old", or a
> boolean value, which will cause it to be "old" (for 'true')
> and 'none' (for 'false').
Wouldn't it be better to use existing 'diff driver' infrastructure
for this? See "Defining a custom hunk-header" section in
gitattributes(5).
On the other hand... no.
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH] diffcore: Allow users to decide what funcname to use
From: Andreas Ericsson @ 2007-11-13 10:07 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <fhbsor$ebf$1@ger.gmane.org>
Jakub Narebski wrote:
> Andreas Ericsson wrote:
>
>> Git can be smarter than that, and imo it should. This
>> patch lets the diffcore grok a new configuration variable,
>> "diff.funcnames", which can be set to "new", "old", or a
>> boolean value, which will cause it to be "old" (for 'true')
>> and 'none' (for 'false').
>
> Wouldn't it be better to use existing 'diff driver' infrastructure
> for this? See "Defining a custom hunk-header" section in
> gitattributes(5).
>
> On the other hand... no.
>
It's impossible to do that, since that driver will only ever be
fed the "old" file with the old code. I'm guessing you noticed
that yourself, so just explaining in case anyone else wonders.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: Cloning empty repositories, was Re: What is the idea for bare repositories?
From: Jakub Narebski @ 2007-11-13 10:08 UTC (permalink / raw)
To: git
In-Reply-To: <vpqzlxiiii6.fsf@bauges.imag.fr>
Matthieu Moy wrote:
> I repeat the use-case I mentionned above :
>
> ,----
> | a typical use-case is when I want to create a new project. I'd
> | like to initialize an empty bare repo on my backed up disk, and then
> | clone it to my local-fast-unreliable disk to get a working copy and do
> | the first commit there.
> `----
>
> I find this quite natural, and up to now, no one gave me either a
> rationale not to do that,
The rationale is that current git just simply cannot do this.
You are welcome to add support for this corner case in git-clone,
or add git protocol extension for symref transfer.
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ 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