* Re: [PATCH 2/3] apply --whitespace: configuration option.
From: Junio C Hamano @ 2006-02-28 9:38 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: git
In-Reply-To: <440414D6.8050407@op5.se>
Andreas Ericsson <ae@op5.se> writes:
> Junio C Hamano wrote:
>> The new configuration option apply.whitespace can take one of
>> "warn", "error", "error-all", or "strip". When git-apply is run
>> to apply the patch to the index, they are used as the default
>> value if there is no command line --whitespace option.
>
> I would think "warn-all" would be the logical thing, since "error"
> either breaks out early or prints all warnings before denying the
> patch anyway.
Actually there is some thinking behind why I did not do warn-all.
I did consider it at first but rejected.
* If you are a busy top echelon person but cares about tree
cleanliness, --whitespace=error is good enough. The patch is
rejected on WS basis whether it introduces one such trailing
WS or hundreds. The patch is returned to the submitter and
the tree remains clean.
* --whitespace=warn-all, if existed, would apply the patch
_anyway_, so if you notice you got warnings, and if that
bothers you enough that you would want to do something about
it, you will have to rewind the HEAD, fix up .dotest/patch
and reapply. This means you are willing to clean up other
peoples' patches.
* But if you are that kind of person, --whitespace=error-all is
a better choice for you. Your tree stays clean and you do
not have to rewind. Instead, you get all the errors you can
go through with your editor (e.g. Emacs users can use C-x `;
I hope vim users have similar macros) and fix things.
* --whitespace=warn would show some, but not all, so that you
can continue while making a mental note to scold the patch
submitter to be careful the next time. You chose "warn" to
apply the patch anyway, so there is no point showing the full
extent of damage -- the damage is already done to your tree.
* --whitespace=strip is for people who care about cleanliness,
who wants to be nice to the submitters, but not nice enough
to educate them. They do not want to fix things by hand.
Instead they have the tool to do the fixing for them.
The last one is somewhat risky, and the output may need to be
examined carefully depending on the contents (e.g. programming
language) the project is dealing with.
^ permalink raw reply
* Re: [PATCH 2/3] apply --whitespace: configuration option.
From: Andreas Ericsson @ 2006-02-28 9:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Andrew Morton
In-Reply-To: <7vacccuvxz.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> The new configuration option apply.whitespace can take one of
> "warn", "error", "error-all", or "strip". When git-apply is run
> to apply the patch to the index, they are used as the default
> value if there is no command line --whitespace option.
>
I would think "warn-all" would be the logical thing, since "error"
either breaks out early or prints all warnings before denying the patch
anyway.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* [PATCH] git-apply: war on whitespace -- finishing touches.
From: Junio C Hamano @ 2006-02-28 9:13 UTC (permalink / raw)
To: git; +Cc: Andrew Morton
In-Reply-To: <20060227153702.375db751.akpm@osdl.org>
Andrew Morton <akpm@osdl.org> writes:
> I'd suggest that git have options to a) generate trailing-whitespace
> warnings, b) generate trailing-whitespace errors and c) strip trailing
> whitespace while applying. And that the as-shipped default be a).
I've done this and will be pushing it out to "master" branch on
my next git day (Wednesday, west coast US); "maint" branch will
have the same for v1.2.4 sometime by the end of this week.
There is one thing. By making --whitespace=warn the default,
the diffstat output people would see after "git pull" would also
show the warning message. I personally do not think this is a
problem (you will know how dirty a tree you are merging into
your tree), but it might not be a bad idea to explicitly squelch
it by making it not to warn when we are not applying.
-- >8 --
This changes the default --whitespace policy to nowarn when we
are only getting --stat, --summary etc. IOW when not applying
the patch. When applying the patch, the default is warn (spit
out warning message but apply the patch).
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
apply.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
f21d6726150ec4219e94ea605f27a4cd58eb3d99
diff --git a/apply.c b/apply.c
index c4ff418..9deb206 100644
--- a/apply.c
+++ b/apply.c
@@ -75,6 +75,15 @@ static void parse_whitespace_option(cons
die("unrecognized whitespace option '%s'", option);
}
+static void set_default_whitespace_mode(const char *whitespace_option)
+{
+ if (!whitespace_option && !apply_default_whitespace) {
+ new_whitespace = (apply
+ ? warn_on_whitespace
+ : nowarn_whitespace);
+ }
+}
+
/*
* For "diff-stat" like behaviour, we keep track of the biggest change
* we've seen, and the longest filename. That allows us to do simple
@@ -1955,9 +1964,11 @@ int main(int argc, char **argv)
if (fd < 0)
usage(apply_usage);
read_stdin = 0;
+ set_default_whitespace_mode(whitespace_option);
apply_patch(fd, arg);
close(fd);
}
+ set_default_whitespace_mode(whitespace_option);
if (read_stdin)
apply_patch(0, "<stdin>");
if (whitespace_error) {
--
1.2.3.g1da2
^ permalink raw reply related
* Re: [PATCH] Add git-annotate, a tool for assigning blame.
From: Andreas Ericsson @ 2006-02-28 9:08 UTC (permalink / raw)
To: Ryan Anderson; +Cc: Fredrik Kuivinen, Junio C Hamano, git
In-Reply-To: <20060228084737.GA13537@mythryan2.michonline.com>
Ryan Anderson wrote:
> On Tue, Feb 28, 2006 at 09:27:36AM +0100, Fredrik Kuivinen wrote:
>
>>On Mon, Feb 20, 2006 at 04:01:56PM -0800, Junio C Hamano wrote:
>>
>>>Fredrik Kuivinen <freku045@student.liu.se> writes:
>>>
>>>
>>>>I have also been working on a blame program.
>>
>>...
>>
>>
>>>BTW, these days I always compile things with
>>>
>>> -Wall -Wdeclaration-after-statement
>>>
>>>which caught quite a many.
>>
>>Just out of curiosity, why do you prefer declarations before
>>statements?
>
>
> I won't speak for Junio, but the explanations I've heard in the past are
> basically:
>
> 1) It keeps all declarations in one spot.
> 2) If your function is complicated enough to not need a variable until
> fairly far into the function, it probably should be two (or more) functions.
>
3) Not all compilers support it.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH] Add git-annotate, a tool for assigning blame.
From: Ryan Anderson @ 2006-02-28 8:47 UTC (permalink / raw)
To: Fredrik Kuivinen; +Cc: Junio C Hamano, git
In-Reply-To: <20060228082736.GA4593@c165.ib.student.liu.se>
On Tue, Feb 28, 2006 at 09:27:36AM +0100, Fredrik Kuivinen wrote:
> On Mon, Feb 20, 2006 at 04:01:56PM -0800, Junio C Hamano wrote:
> > Fredrik Kuivinen <freku045@student.liu.se> writes:
> >
> > > I have also been working on a blame program.
>
> ...
>
> > BTW, these days I always compile things with
> >
> > -Wall -Wdeclaration-after-statement
> >
> > which caught quite a many.
>
> Just out of curiosity, why do you prefer declarations before
> statements?
I won't speak for Junio, but the explanations I've heard in the past are
basically:
1) It keeps all declarations in one spot.
2) If your function is complicated enough to not need a variable until
fairly far into the function, it probably should be two (or more) functions.
Basically, I think that there's not wrong with doing it that way, per
se, just that it's sometimes a symptom of other problems, so fi you look
for the symptom, the problem sometimes is more obvious.
--
Ryan Anderson
sometimes Pug Majere
^ permalink raw reply
* Re: [PATCH] Add git-annotate, a tool for assigning blame.
From: Junio C Hamano @ 2006-02-28 8:38 UTC (permalink / raw)
To: Fredrik Kuivinen; +Cc: git
In-Reply-To: <20060228082736.GA4593@c165.ib.student.liu.se>
Fredrik Kuivinen <freku045@student.liu.se> writes:
> On Mon, Feb 20, 2006 at 04:01:56PM -0800, Junio C Hamano wrote:
>>
>> BTW, these days I always compile things with
>>
>> -Wall -Wdeclaration-after-statement
>>
>> which caught quite a many.
>
> Just out of curiosity, why do you prefer declarations before
> statements?
Inertia, IOW, mostly being used to read code written that way.
^ permalink raw reply
* Re: [PATCH] Add git-annotate, a tool for assigning blame.
From: Fredrik Kuivinen @ 2006-02-28 8:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Fredrik Kuivinen, git, Ryan Anderson
In-Reply-To: <7vlkw57f63.fsf@assigned-by-dhcp.cox.net>
On Mon, Feb 20, 2006 at 04:01:56PM -0800, Junio C Hamano wrote:
> Fredrik Kuivinen <freku045@student.liu.se> writes:
>
> > I have also been working on a blame program.
...
> BTW, these days I always compile things with
>
> -Wall -Wdeclaration-after-statement
>
> which caught quite a many.
Just out of curiosity, why do you prefer declarations before
statements?
- Fredrik
^ permalink raw reply
* Re: [PATCH] diff-delta: bound hash list length to avoid O(m*n) behavior
From: Junio C Hamano @ 2006-02-28 8:10 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git
In-Reply-To: <7vhd6kq8lc.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> Nicolas Pitre <nico@cam.org> writes:
>
>> blob 9af06ba723df75fed49f7ccae5b6c9c34bc5115f ->
>> blob dfc9cd58dc065d17030d875d3fea6e7862ede143
>> size (491102 -> 496045)
>> delta size (16 byte blocks): 248899 in less than 0.1 sec
>> delta size (3 byte blocks): 136000 in 11.8 secs
>> delta size (3 byte blocks + this patch): 171688 in 0.79 sec
>
> These numbers are misleading.
>
> The 36K objects pack I used in my previous tests gives 9971223
> (from "next" branch) vs 9664546 (this patch) final pack size.
> The wallclock time on my machine is 1m35 vs 3m30. I doubt many
> people are willing to pay 100% more waiting time for 3% tighter
> pack.
I tried an experimental patch to cull collided hash buckets
very aggressively. I haven't applied your last "reuse index"
patch, though -- I think that is orthogonal and I'd like to
leave that to the next round.
With the same dataset: resulting pack is 9651096 vs 9664546
(your patch) final pack size, with wallclock 2m45s (user 2m31).
Still not good enough, and at the same time I wonder why it gets
_smaller_ results than yours. But the generated pack unpacked
cleanly in a cloned linux-2.6 repository (having objects and
refs up to v2.6.14) and the result was fsck-objects clean.
I'd appreciate it if you can test it on the 20MB blobs and see
what happens if you have time.
BTW, the benchmark I've been doing is with this dataset:
git rev-list --objects-edge v2.6.14..v2.6.15-rc1 >RL-N
time git pack-objects <RL-N --stdout | wc -c
---
diff --git a/diff-delta.c b/diff-delta.c
index 0730b24..52df372 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -88,22 +88,35 @@ static struct index ** delta_index(const
/*
* Now make sure none of the hash buckets has more entries than
- * we're willing to test. Otherwise we short-circuit the entry
- * list uniformly to still preserve a good repartition across
- * the reference buffer.
+ * we're willing to test. Otherwise we cull the entry list to
+ * limit identical three byte prefixes to still preserve a good
+ * repartition across the reference buffer.
*/
for (i = 0; i < hsize; i++) {
+ struct index **list, *bucket, *remaining;
+ int cnt;
if (hash_count[i] < hlimit)
continue;
- entry = hash[i];
- do {
- struct index *keep = entry;
- int skip = hash_count[i] / hlimit / 2;
- do {
- entry = entry->next;
- } while(--skip && entry);
- keep->next = entry;
- } while(entry);
+
+ bucket = NULL;
+ list = &bucket;
+ remaining = hash[i];
+ cnt = 0;
+ while (cnt < hlimit && remaining) {
+ struct index *this = remaining, *that;
+ remaining = remaining->next;
+ for (that = bucket; that; that = that->next) {
+ if (!memcmp(that->ptr, this->ptr, 3))
+ break;
+ }
+ if (that)
+ continue; /* discard */
+ cnt++;
+ *list = this;
+ list = &(this->next);
+ this->next = NULL;
+ }
+ hash[i] = bucket;
}
free(hash_count);
^ permalink raw reply related
* Re: [PATCH] diff-delta: bound hash list length to avoid O(m*n) behavior
From: Junio C Hamano @ 2006-02-28 6:51 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0602272110320.25336@localhost.localdomain>
Nicolas Pitre <nico@cam.org> writes:
> The diff-delta code can exhibit O(m*n) behavior with some patological
> data set where most hash entries end up in the same hash bucket.
> Other data points from the Linux kernel repository:
>
> blob 9af06ba723df75fed49f7ccae5b6c9c34bc5115f ->
> blob dfc9cd58dc065d17030d875d3fea6e7862ede143
> size (491102 -> 496045)
> delta size (16 byte blocks): 248899 in less than 0.1 sec
> delta size (3 byte blocks): 136000 in 11.8 secs
> delta size (3 byte blocks + this patch): 171688 in 0.79 sec
These numbers are misleading.
The 36K objects pack I used in my previous tests gives 9971223
(from "next" branch) vs 9664546 (this patch) final pack size.
The wallclock time on my machine is 1m35 vs 3m30. I doubt many
people are willing to pay 100% more waiting time for 3% tighter
pack.
Although I do not mean to rain on your effort and substantial
improvement you made with this patch, we need to admit that
improving pathological corner case has quite a diminishing
effect in the overall picture.
But this definitely is an improvement nevertheless. I should
try this on my wife's AMD64 (Cygwin). The same datasets I used
in my previous complaint still seem to take a couple of seconds
(or more) each on my Duron 750 X-<.
A handful additional ideas.
* Lower the hash limit a bit more (obvious). That might have
detrimental effect in the general case.
* Study the hash bucket distribution for the pathological case
and see if we can cheaply detect a pattern. I suspect these
cases have relatively few but heavily collided buckets with
mostly sparse other entries. If there is such an easily
detectable pattern, maybe we can look for such a pattern at
runtime, and cull hash entries more aggressively in such a
case?
* Try checking the target buffer to see if it would have many
hits on the heavily collided hash entries from the source
(maybe hash_count the target buffer as well).
* Have pack-object detect a pathological blob (the test patch I
sent you previously uses the eye-candy timer for this
purpose, but we could getrusage() if we want to be more
precise) by observing how much time is spent for a single
round, and mark the blob as such, so we do not delta against
it with other blobs in find_deltas, when we are packing many
objects. It does not really matter in the big picture if we
choose not to delta the pathological ones tightly, as long as
they are relatively few.
* Also in pack-object, have an optional backing-store to write
out deltified representations for results that took more than
certain amount of time to produce in find_deltas(), and reuse
them in write_object().
If anybody is interested, here are some other interesting pairs
from the Linux 2.6 repositories.
2645d6239b74 cb968e7a0fcd
357980942d73 5bb837052ef1
5412dcb4b6d0 ac07e18abb1d
5bb837052ef1 1f8744ad9cde
63d827d7da07 5bb837052ef1
9af06ba723df 1896a0eea7e5
cb968e7a0fcd 97b5578ae9b1
d8c5a8dbd086 97b5578ae9b1
d8c5a8dbd086 cb968e7a0fcd
dfc9cd58dc06 1896a0eea7e5
fa0f82ec9fa0 b611e27470e1
^ permalink raw reply
* Re: someone changed the contents of my HEAD.
From: Dave Jones @ 2006-02-28 5:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlkvwt5v9.fsf@assigned-by-dhcp.cox.net>
On Mon, Feb 27, 2006 at 09:22:18PM -0800, Junio C Hamano wrote:
> > #!/bin/sh
> > export GIT_AUTHOR_NAME="$1"
> > export GIT_AUTHOR_EMAIL="$2"
> > tree=$(git-write-tree) || exit 1
> > commit=$(git-commit-tree $tree -p HEAD) || exit 1
> > echo $commit > .git/HEAD
>
> This has been deprecated for a looong time, but perhaps I should
> have been louder. "git commit --author" should be fine -- I do
> not think you do not even need such a wrapper.
I guess I wasn't paying attention :)
It's worked just fine up until a few days ago.
I can change my habits to use the preferred approach though.
> > For my newly created repos, this isn't a problem, as I can fudge my
> > commit-as script to write to .git/refs/heads/master instead, but
> > my concern now is the unpulled changes in the existing repos
> > I have on master. Will Linus be able to pull those into his tree
> > with git 1.2.3, or will I have to recreate those repos with the
> > new-style .git/HEAD ?
>
> A .git/HEAD symlink pointing at refs/heads/master _is_ still
> (and will be) supported, so either symlink or symref is fine.
> Bare SHA1 object name in HEAD is not -- git would not know which
> branch you are on, so "checkin" would not work after that.
>
> Your refs/heads/master might be stale (one rev or more older
> than what you replaced .git/HEAD with) but still should be a
> proper ancestor. After making sure your refs/heads/master has
> the right commit (you may have it there already if you have been
> updating them using "git-push"), running "git-symbolic-ref HEAD
> refs/heads/master" would fix things.
One scary thing fell out of this. Andrew did a pull on my trees
and found bogons in both of them. Both agpgart and cpufreq trees
contained partial (1 file) reverts of earlier commits in my last commit
to the head of each repo. I can't explain how that happened, and
I'm surprised my head-munging would be the reason.
I've just rebuilt those two repos as there was only 3-4 patches
in both, so it wasn't a big deal. Something to watch out for
though if anyone else has been doing something similar to how
I was doing checkins.
Dave
^ permalink raw reply
* Re: [PATCH 2/2] Don't use excessive non-standard border width
From: Aneesh Kumar K.V @ 2006-02-28 5:53 UTC (permalink / raw)
To: Pavel Roskin
In-Reply-To: <1141104931.22029.5.camel@dv>
On Tue, Feb 28, 2006 at 12:35:31AM -0500, Pavel Roskin wrote:
> Hi, Aneesh!
>
> On Tue, 2006-02-28 at 10:46 +0530, Aneesh Kumar K.V wrote:
> > I am not sure about this. I guess it looks much better with a border.
> >
> > Not applied.
>
> I think what really annoyed me was the menu with lone "Help" on the
> right hand side, and excessive borders only made it look worse. Maybe
> the border should not be used on the menu? It looks wrong to me that
> the menu is appended to the top pane rather than to the whole window.
>
how about this
diff --git a/contrib/gitview/gitview b/contrib/gitview/gitview
index aded7ed..e57c53b 100755
--- a/contrib/gitview/gitview
+++ b/contrib/gitview/gitview
@@ -482,19 +482,10 @@ class GitView:
def construct(self):
"""Construct the window contents."""
+ vbox = gtk.VBox()
paned = gtk.VPaned()
paned.pack1(self.construct_top(), resize=False, shrink=True)
paned.pack2(self.construct_bottom(), resize=False, shrink=True)
- self.window.add(paned)
- paned.show()
-
-
- def construct_top(self):
- """Construct the top-half of the window."""
- vbox = gtk.VBox(spacing=6)
- vbox.set_border_width(12)
- vbox.show()
-
menu_bar = gtk.MenuBar()
menu_bar.set_pack_direction(gtk.PACK_DIRECTION_RTL)
help_menu = gtk.MenuItem("Help")
@@ -506,8 +497,20 @@ class GitView:
help_menu.set_submenu(menu)
help_menu.show()
menu_bar.append(help_menu)
- vbox.pack_start(menu_bar, False, False, 2)
menu_bar.show()
+ vbox.pack_start(menu_bar, False, False, 2)
+ vbox.pack_start(paned, True, True, 2)
+ self.window.add(vbox)
+ paned.show()
+ vbox.show()
+
+
+ def construct_top(self):
+ """Construct the top-half of the window."""
+ vbox = gtk.VBox(spacing=6)
+ vbox.set_border_width(12)
+ vbox.show()
+
scrollwin = gtk.ScrolledWindow()
scrollwin.set_policy(gtk.POLICY_NEVER, gtk.POLICY_AUTOMATIC)
^ permalink raw reply related
* Re: [PATCH 2/2] Don't use excessive non-standard border width
From: Pavel Roskin @ 2006-02-28 5:35 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: git
In-Reply-To: <20060228051611.GB5934@satan.india.hp.com>
Hi, Aneesh!
On Tue, 2006-02-28 at 10:46 +0530, Aneesh Kumar K.V wrote:
> I am not sure about this. I guess it looks much better with a border.
>
> Not applied.
I think what really annoyed me was the menu with lone "Help" on the
right hand side, and excessive borders only made it look worse. Maybe
the border should not be used on the menu? It looks wrong to me that
the menu is appended to the top pane rather than to the whole window.
--
Regards,
Pavel Roskin
^ permalink raw reply
* Re: someone changed the contents of my HEAD.
From: Junio C Hamano @ 2006-02-28 5:22 UTC (permalink / raw)
To: Dave Jones; +Cc: git
In-Reply-To: <20060228030446.GA23490@redhat.com>
Dave Jones <davej@redhat.com> writes:
> When checking in changes previously, I used this..
>
> #!/bin/sh
> export GIT_AUTHOR_NAME="$1"
> export GIT_AUTHOR_EMAIL="$2"
> tree=$(git-write-tree) || exit 1
> commit=$(git-commit-tree $tree -p HEAD) || exit 1
> echo $commit > .git/HEAD
This has been deprecated for a looong time, but perhaps I should
have been louder. "git commit --author" should be fine -- I do
not think you do not even need such a wrapper.
> For my newly created repos, this isn't a problem, as I can fudge my
> commit-as script to write to .git/refs/heads/master instead, but
> my concern now is the unpulled changes in the existing repos
> I have on master. Will Linus be able to pull those into his tree
> with git 1.2.3, or will I have to recreate those repos with the
> new-style .git/HEAD ?
A .git/HEAD symlink pointing at refs/heads/master _is_ still
(and will be) supported, so either symlink or symref is fine.
Bare SHA1 object name in HEAD is not -- git would not know which
branch you are on, so "checkin" would not work after that.
Your refs/heads/master might be stale (one rev or more older
than what you replaced .git/HEAD with) but still should be a
proper ancestor. After making sure your refs/heads/master has
the right commit (you may have it there already if you have been
updating them using "git-push"), running "git-symbolic-ref HEAD
refs/heads/master" would fix things.
^ permalink raw reply
* Re: [PATCH 2/2] Don't use excessive non-standard border width
From: Aneesh Kumar K.V @ 2006-02-28 5:16 UTC (permalink / raw)
To: Pavel Roskin; +Cc: git
In-Reply-To: <20060228045631.21880.27670.stgit@dv.roinet.com>
On Mon, Feb 27, 2006 at 11:56:31PM -0500, Pavel Roskin wrote:
> From: Pavel Roskin <proski@gnu.org>
>
>
> ---
>
> contrib/gitview/gitview | 2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/gitview/gitview b/contrib/gitview/gitview
> index aded7ed..00cb8ee 100755
> --- a/contrib/gitview/gitview
> +++ b/contrib/gitview/gitview
> @@ -492,7 +492,6 @@ class GitView:
> def construct_top(self):
> """Construct the top-half of the window."""
> vbox = gtk.VBox(spacing=6)
> - vbox.set_border_width(12)
> vbox.show()
>
> menu_bar = gtk.MenuBar()
> @@ -574,7 +573,6 @@ class GitView:
> def construct_bottom(self):
> """Construct the bottom half of the window."""
> vbox = gtk.VBox(False, spacing=6)
> - vbox.set_border_width(12)
> (width, height) = self.window.get_size()
> vbox.set_size_request(width, int(height / 2.5))
> vbox.show()
I am not sure about this. I guess it looks much better with a border.
Not applied.
-aneesh
^ permalink raw reply
* Re: [PATCH 1/2] Use solid text for refs in gitview
From: Aneesh Kumar K.V @ 2006-02-28 5:15 UTC (permalink / raw)
To: Pavel Roskin; +Cc: git
In-Reply-To: <20060228045629.21880.19007.stgit@dv.roinet.com>
On Mon, Feb 27, 2006 at 11:56:29PM -0500, Pavel Roskin wrote:
> From: Pavel Roskin <proski@gnu.org>
>
> Select the text color based on whether the entry in highlighted. Use
> standard font.
> ---
Applied
-aneesh
^ permalink raw reply
* Re: [PATCH 3/3] git-apply --whitespace=nowarn
From: Junio C Hamano @ 2006-02-28 5:08 UTC (permalink / raw)
To: A Large Angry SCM; +Cc: git, Andrew Morton
In-Reply-To: <4403C303.70602@gmail.com>
A Large Angry SCM <gitzilla@gmail.com> writes:
> Junio C Hamano wrote:
>> Andrew insists --whitespace=warn should be the default, and I
>> tend to agree. This introduces --whitespace=warn, so if your
>> project policy is more lenient, you can squelch them by having
>> apply.whitespace=nowarn in your configuration file.
>> Signed-off-by: Junio C Hamano <junkio@cox.net>
>
> I think this is wrong. The default policy of, non-security, tools
> SHOULD be the least restrictive and most flexible.
The reason is that --whitespace=warn does not refuse to apply
but spits out some warning messages. Admittedly, going from
zero, any amount of new warning message makes it infinitely more
chatty, but then people can squelch it by having the
configuration option "apply.whitespace = nowarn".
^ permalink raw reply
* [PATCH] diff-delta: allow reusing of the reference buffer index
From: Nicolas Pitre @ 2006-02-28 4:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
When a reference buffer is used multiple times then its index can be
computed only once and reused multiple times. This patch adds an extra
pointer to a pointer argument (from_index) to diff_delta() for this.
If from_index is NULL then everything is like before.
If from_index is non NULL and *from_index is NULL then the index is
created and its location stored to *from_index. In this case the caller
has the responsibility to free the memory pointed to by *from_index.
If from_index and *from_index are non NULL then the index is reused as
is.
This currently saves about 10% of CPU time to repack the git archive.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
This is partly what Linus suggested recently. However I only made the
minimum changes to pack-objects.c to try it out and demonstrate how it
works. I however don't feel confortable enough with the changes
required to implement the full solution which is to turn the window
processing around i.e. keep the reference buffer stable while different
targets are tested against this reference. This is something worth
doing since that would reduce memory usage significantly. But Like I
said, pack-objects as grown more complex with Junio latest additions
so...
delta.h | 3 ++-
diff-delta.c | 41 +++++++++++++++++++++++++++--------------
diffcore-break.c | 2 +-
diffcore-rename.c | 2 +-
pack-objects.c | 11 ++++++++---
test-delta.c | 2 +-
6 files changed, 40 insertions(+), 21 deletions(-)
f660c44250f5c02a7e773ed991316f7f16950f3e
diff --git a/delta.h b/delta.h
index a15350d..00fef0b 100644
--- a/delta.h
+++ b/delta.h
@@ -4,7 +4,8 @@
/* handling of delta buffers */
extern void *diff_delta(void *from_buf, unsigned long from_size,
void *to_buf, unsigned long to_size,
- unsigned long *delta_size, unsigned long max_size);
+ unsigned long *delta_size, unsigned long max_size,
+ void **from_index);
extern void *patch_delta(void *src_buf, unsigned long src_size,
void *delta_buf, unsigned long delta_size,
unsigned long *dst_size);
diff --git a/diff-delta.c b/diff-delta.c
index bb07926..67561cd 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -30,8 +30,7 @@ struct index {
static struct index ** delta_index(const unsigned char *buf,
unsigned long bufsize,
- unsigned long trg_bufsize,
- unsigned int *hash_shift)
+ unsigned long trg_bufsize)
{
unsigned long hsize;
unsigned int i, hshift, hlimit, *hash_count;
@@ -44,14 +43,17 @@ static struct index ** delta_index(const
for (i = 8; (1 << i) < hsize && i < 24; i += 2);
hsize = 1 << i;
hshift = (i - 8) / 2;
- *hash_shift = hshift;
- /* allocate lookup index */
- mem = malloc(hsize * sizeof(*hash) + bufsize * sizeof(*entry));
+ /*
+ * Allocate lookup index. Note the first hash pointer
+ * is used to store the hash shift value.
+ */
+ mem = malloc((1 + hsize) * sizeof(*hash) + bufsize * sizeof(*entry));
if (!mem)
return NULL;
hash = mem;
- entry = mem + hsize * sizeof(*hash);
+ *hash++ = (void *)hshift;
+ entry = mem + (1 + hsize) * sizeof(*hash);
memset(hash, 0, hsize * sizeof(*hash));
/* allocate an array to count hash entries */
@@ -107,7 +109,7 @@ static struct index ** delta_index(const
}
free(hash_count);
- return hash;
+ return hash-1;
}
/* provide the size of the copy opcode given the block offset and size */
@@ -121,7 +123,8 @@ static struct index ** delta_index(const
void *diff_delta(void *from_buf, unsigned long from_size,
void *to_buf, unsigned long to_size,
unsigned long *delta_size,
- unsigned long max_size)
+ unsigned long max_size,
+ void **from_index)
{
unsigned int i, outpos, outsize, inscnt, hash_shift;
const unsigned char *ref_data, *ref_top, *data, *top;
@@ -130,9 +133,16 @@ void *diff_delta(void *from_buf, unsigne
if (!from_size || !to_size)
return NULL;
- hash = delta_index(from_buf, from_size, to_size, &hash_shift);
- if (!hash)
- return NULL;
+ if (from_index && *from_index) {
+ hash = *from_index;
+ } else {
+ hash = delta_index(from_buf, from_size, to_size);
+ if (!hash)
+ return NULL;
+ if (from_index)
+ *from_index = hash;
+ }
+ hash_shift = (unsigned int)(*hash++);
outpos = 0;
outsize = 8192;
@@ -140,7 +150,8 @@ void *diff_delta(void *from_buf, unsigne
outsize = max_size + MAX_OP_SIZE + 1;
out = malloc(outsize);
if (!out) {
- free(hash);
+ if (!from_index)
+ free(hash-1);
return NULL;
}
@@ -241,7 +252,8 @@ void *diff_delta(void *from_buf, unsigne
out = realloc(out, outsize);
if (!out) {
free(tmp);
- free(hash);
+ if (!from_index)
+ free(hash-1);
return NULL;
}
}
@@ -250,7 +262,8 @@ void *diff_delta(void *from_buf, unsigne
if (inscnt)
out[outpos - inscnt - 1] = inscnt;
- free(hash);
+ if (!from_index)
+ free(hash-1);
*delta_size = outpos;
return out;
}
diff --git a/diffcore-break.c b/diffcore-break.c
index c57513a..6dc22d5 100644
--- a/diffcore-break.c
+++ b/diffcore-break.c
@@ -67,7 +67,7 @@ static int should_break(struct diff_file
delta = diff_delta(src->data, src->size,
dst->data, dst->size,
- &delta_size, 0);
+ &delta_size, 0, NULL);
if (!delta)
return 0; /* error but caught downstream */
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 39d9126..5b4cde4 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -168,7 +168,7 @@ static int estimate_similarity(struct di
delta_limit = base_size * (MAX_SCORE-minimum_score) / MAX_SCORE;
delta = diff_delta(src->data, src->size,
dst->data, dst->size,
- &delta_size, delta_limit);
+ &delta_size, delta_limit, NULL);
if (!delta)
/* If delta_limit is exceeded, we have too much differences */
return 0;
diff --git a/pack-objects.c b/pack-objects.c
index ceb107f..2b2d9a9 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -202,7 +202,7 @@ static void *delta_against(void *buf, un
if (!otherbuf)
die("unable to read %s", sha1_to_hex(entry->delta->sha1));
delta_buf = diff_delta(otherbuf, othersize,
- buf, size, &delta_size, 0);
+ buf, size, &delta_size, 0, NULL);
if (!delta_buf || delta_size != entry->delta_size)
die("delta size changed");
free(buf);
@@ -707,6 +707,7 @@ static int type_size_sort(const struct o
struct unpacked {
struct object_entry *entry;
void *data;
+ void **delta_index;
};
/*
@@ -789,7 +790,8 @@ static int try_delta(struct unpacked *cu
if (sizediff >= max_size)
return -1;
delta_buf = diff_delta(old->data, oldsize,
- cur->data, size, &delta_size, max_size);
+ cur->data, size, &delta_size,
+ max_size, old->delta_index);
if (!delta_buf)
return 0;
cur_entry->delta = old_entry;
@@ -830,6 +832,7 @@ static void find_deltas(struct object_en
*/
continue;
+ free(n->delta_index);
free(n->data);
n->entry = entry;
n->data = read_sha1_file(entry->sha1, type, &size);
@@ -853,8 +856,10 @@ static void find_deltas(struct object_en
idx = 0;
}
- for (i = 0; i < window; ++i)
+ for (i = 0; i < window; ++i) {
+ free(array[i].delta_index);
free(array[i].data);
+ }
free(array);
}
diff --git a/test-delta.c b/test-delta.c
index 1be8ee0..89eb68e 100644
--- a/test-delta.c
+++ b/test-delta.c
@@ -63,7 +63,7 @@ int main(int argc, char *argv[])
if (argv[1][1] == 'd')
out_buf = diff_delta(from_buf, from_size,
data_buf, data_size,
- &out_size, 0);
+ &out_size, 0, NULL);
else
out_buf = patch_delta(from_buf, from_size,
data_buf, data_size,
--
1.2.3.g8fcf1-dirty
^ permalink raw reply related
* [PATCH] diff-delta: bound hash list length to avoid O(m*n) behavior
From: Nicolas Pitre @ 2006-02-28 4:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
The diff-delta code can exhibit O(m*n) behavior with some patological
data set where most hash entries end up in the same hash bucket.
The latest code rework reduced the block size making it particularly
vulnerable to this issue, but the issue was always there and can be
triggered regardless of the block size.
This patch does two things:
1) the hashing has been reworked to offer a better distribution to
atenuate the problem a bit, and
2) a limit is imposed to the number of entries that can exist in the
same hash bucket.
Because of the above the code is a bit more expensive on average, but
the problematic samples used to diagnoze the issue are now orders of
magnitude less expensive to process with only a slight loss in
compression.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
For example, Carl Baldwin provided me with a couple 20MB files, and
deltifying one against another one with test-delta takes around
SEVENTEEN MINUTES for only one delta on my P4 @ 3GHz when using the
original adler32 version with 16 byte blocks. With the latest delta
code in the pu branch it simply takes forever (I interrupted it after 20
minutes of processing). now imagine using git-repack -a with a window
of 10. With this patch on top of the small block patch (still in the pu
branch) this time dropped to only 10 seconds. And the resulting delta
is still smaller than the 16 byte adler32 based version:
blob 02220dae8cd219916ce52a12cda67322659e0e57 ->
blob af8f99dd11ca288f4e4a08f2626af2de8b3ecfd2
size (21187748 -> 19424744)
delta size (16 byte blocks): 5238542 in 16m55
delta size (3 byte blocks): [interrupted after 20 minutes]
delta size (3 byte blocks + this patch): 4910988 in 9.69 secs
Other data points from the Linux kernel repository:
blob 9af06ba723df75fed49f7ccae5b6c9c34bc5115f ->
blob dfc9cd58dc065d17030d875d3fea6e7862ede143
size (491102 -> 496045)
delta size (16 byte blocks): 248899 in less than 0.1 sec
delta size (3 byte blocks): 136000 in 11.8 secs
delta size (3 byte blocks + this patch): 171688 in 0.79 sec
blob 4917ec509720a42846d513addc11cbd25e0e3c4f ->
blob dfc9cd58dc065d17030d875d3fea6e7862ede143
size (495831 -> 496045)
delta size (16 byte blocks): 226218 in less than 0.1 sec
delta size (3 byte blocks): 120948 in 11.7 secs
delta size (3 byte blocks + this patch): 157135 in 0.77 sec
diff-delta.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++-----------
1 files changed, 56 insertions(+), 13 deletions(-)
1682f4b1b2899288d7761844a4cfd02772c464d1
diff --git a/diff-delta.c b/diff-delta.c
index 27f83a0..bb07926 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -30,19 +30,20 @@ struct index {
static struct index ** delta_index(const unsigned char *buf,
unsigned long bufsize,
+ unsigned long trg_bufsize,
unsigned int *hash_shift)
{
unsigned long hsize;
- unsigned int hshift, i;
+ unsigned int i, hshift, hlimit, *hash_count;
const unsigned char *data;
struct index *entry, **hash;
void *mem;
/* determine index hash size */
hsize = bufsize / 4;
- for (i = 8; (1 << i) < hsize && i < 16; i++);
+ for (i = 8; (1 << i) < hsize && i < 24; i += 2);
hsize = 1 << i;
- hshift = i - 8;
+ hshift = (i - 8) / 2;
*hash_shift = hshift;
/* allocate lookup index */
@@ -53,15 +54,59 @@ static struct index ** delta_index(const
entry = mem + hsize * sizeof(*hash);
memset(hash, 0, hsize * sizeof(*hash));
- /* then populate it */
+ /* allocate an array to count hash entries */
+ hash_count = calloc(hsize, sizeof(*hash_count));
+ if (!hash_count) {
+ free(hash);
+ return NULL;
+ }
+
+ /* then populate the index */
data = buf + bufsize - 2;
while (data > buf) {
entry->ptr = --data;
- i = data[0] ^ data[1] ^ (data[2] << hshift);
+ i = data[0] ^ ((data[1] ^ (data[2] << hshift)) << hshift);
entry->next = hash[i];
hash[i] = entry++;
+ hash_count[i]++;
}
+ /*
+ * Determine a limit on the number of entries in the same hash
+ * bucket. This guard us against patological data sets causing
+ * really bad hash distribution with most entries in the same hash
+ * bucket that would bring us to O(m*n) computing costs (m and n
+ * corresponding to reference and target buffer sizes).
+ *
+ * The more the target buffer is large, the more it is important to
+ * have small entry lists for each hash buckets. With such a limit
+ * the cost is bounded to something more like O(m+n).
+ */
+ hlimit = (1 << 26) / trg_bufsize;
+ if (hlimit < 16)
+ hlimit = 16;
+
+ /*
+ * Now make sure none of the hash buckets has more entries than
+ * we're willing to test. Otherwise we short-circuit the entry
+ * list uniformly to still preserve a good repartition across
+ * the reference buffer.
+ */
+ for (i = 0; i < hsize; i++) {
+ if (hash_count[i] < hlimit)
+ continue;
+ entry = hash[i];
+ do {
+ struct index *keep = entry;
+ int skip = hash_count[i] / hlimit / 2;
+ do {
+ entry = entry->next;
+ } while(--skip && entry);
+ keep->next = entry;
+ } while(entry);
+ }
+ free(hash_count);
+
return hash;
}
@@ -85,7 +130,7 @@ void *diff_delta(void *from_buf, unsigne
if (!from_size || !to_size)
return NULL;
- hash = delta_index(from_buf, from_size, &hash_shift);
+ hash = delta_index(from_buf, from_size, to_size, &hash_shift);
if (!hash)
return NULL;
@@ -126,8 +171,8 @@ void *diff_delta(void *from_buf, unsigne
while (data < top) {
unsigned int moff = 0, msize = 0;
- if (data + 2 < top) {
- i = data[0] ^ data[1] ^ (data[2] << hash_shift);
+ if (data + 3 <= top) {
+ i = data[0] ^ ((data[1] ^ (data[2] << hash_shift)) << hash_shift);
for (entry = hash[i]; entry; entry = entry->next) {
const unsigned char *ref = entry->ptr;
const unsigned char *src = data;
@@ -138,11 +183,9 @@ void *diff_delta(void *from_buf, unsigne
ref_size = 0x10000;
if (ref_size <= msize)
break;
- while (ref_size && *src++ == *ref) {
- ref++;
- ref_size--;
- }
- ref_size = ref - entry->ptr;
+ if (*ref != *src)
+ continue;
+ while (ref_size-- && *++src == *++ref);
if (msize < ref - entry->ptr) {
/* this is our best match so far */
msize = ref - entry->ptr;
--
1.2.3.g8fcf1-dirty
^ permalink raw reply related
* Re: [PATCH 3/3] git-apply --whitespace=nowarn
From: A Large Angry SCM @ 2006-02-28 3:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Andrew Morton
In-Reply-To: <7v4q2kuvxk.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> Andrew insists --whitespace=warn should be the default, and I
> tend to agree. This introduces --whitespace=warn, so if your
> project policy is more lenient, you can squelch them by having
> apply.whitespace=nowarn in your configuration file.
>
> Signed-off-by: Junio C Hamano <junkio@cox.net>
I think this is wrong. The default policy of, non-security, tools SHOULD
be the least restrictive and most flexible.
^ permalink raw reply
* someone changed the contents of my HEAD.
From: Dave Jones @ 2006-02-28 3:04 UTC (permalink / raw)
To: git
I just tried to check in some changes to some trees
on master.kernel.org, and found after the first checkin
that git claimed..
fatal: Not a git repository
A lot of head-scratching later, I think I've figured out
what's happened. It seems there was a recent upgrade
to the version of git on m.k.o, which is incompatible
with the helper scripts I used before.
When checking in changes previously, I used this..
#!/bin/sh
export GIT_AUTHOR_NAME="$1"
export GIT_AUTHOR_EMAIL="$2"
tree=$(git-write-tree) || exit 1
commit=$(git-commit-tree $tree -p HEAD) || exit 1
echo $commit > .git/HEAD
and called it thus..
commit-as "Dave Jones" "<davej@redhat.com>"
Previously, this updated .git/HEAD to a ptr to the latest committed change.
All was well, as I only ever have one HEAD in my trees.
With the new .git however, when I clone a new tree, .git/HEAD
contains ref: refs/heads/master, so my script destroys the git metadata.
For my newly created repos, this isn't a problem, as I can fudge my
commit-as script to write to .git/refs/heads/master instead, but
my concern now is the unpulled changes in the existing repos
I have on master. Will Linus be able to pull those into his tree
with git 1.2.3, or will I have to recreate those repos with the
new-style .git/HEAD ?
Dave
^ permalink raw reply
* Re: Quick question: how to generate a patch?
From: Aubrey @ 2006-02-28 2:48 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Andreas Ericsson, git
In-Reply-To: <Pine.LNX.4.64.0602271802110.22647@g5.osdl.org>
> If you can re-create this at will, it would be interesting to see _what_
> makes git think you've modified a file. A small mod to "read-cache.c" like
> the appended patch should give you (very very verbose) output that could
> help us figure this out.
Oops, It's my fault. My mistake is that copy the clone git directory
to another directory for backup. And work on another directory. Sorry
for my incaution.
-Aubrey
^ permalink raw reply
* Re: Quick question: how to generate a patch?
From: Linus Torvalds @ 2006-02-28 2:09 UTC (permalink / raw)
To: Aubrey; +Cc: Andreas Ericsson, git
In-Reply-To: <6d6a94c50602271755v41e0d31ch25892192547003a9@mail.gmail.com>
On Tue, 28 Feb 2006, Aubrey wrote:
>
> I'm using suse9.3. The filesystem is EXT3.
Ok, something else is going on. There's no way ext3 can get confused about
times that I can see.
> I think I forgot one thing last night. When I changed the file, I
> compiled the package to verify my modification. It should be the
> reason. But should it really affect the result of "git diff"?
Nope. Something else is happening.
If you can re-create this at will, it would be interesting to see _what_
makes git think you've modified a file. A small mod to "read-cache.c" like
the appended patch should give you (very very verbose) output that could
help us figure this out.
Linus
----
diff --git a/read-cache.c b/read-cache.c
index f97f92d..4946163 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -183,6 +183,8 @@ int ce_match_stat(struct cache_entry *ce
index_file_timestamp <= ntohl(ce->ce_mtime.sec))
changed |= ce_modified_check_fs(ce, st);
+if (changed) error("changed: 0x%x %s", changed, ce->name);
+
return changed;
}
^ permalink raw reply related
* Re: Quick question: how to generate a patch?
From: Aubrey @ 2006-02-28 1:55 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Andreas Ericsson, git
In-Reply-To: <Pine.LNX.4.64.0602270923120.22647@g5.osdl.org>
On 2/28/06, Linus Torvalds <torvalds@osdl.org> wrote:
> I could well imagine that we still have some bug like that (ctime in
> particular is much less used than mtime, and thus more likely to have not
> been noticed). And it could be much worse on some less-commonly-used and
> less-unixy networked filesystem like smb, which is why I was wondering
> what OS version and filesystem Aubrey might be using.
I'm using suse9.3. The filesystem is EXT3.
I think I forgot one thing last night. When I changed the file, I
compiled the package to verify my modification. It should be the
reason. But should it really affect the result of "git diff"?
Thanks,
-Aubrey
^ permalink raw reply
* [PATCH 3/3] git-apply --whitespace=nowarn
From: Junio C Hamano @ 2006-02-28 1:13 UTC (permalink / raw)
To: git; +Cc: Andrew Morton
In-Reply-To: <7vhd6kxuea.fsf@assigned-by-dhcp.cox.net>
Andrew insists --whitespace=warn should be the default, and I
tend to agree. This introduces --whitespace=warn, so if your
project policy is more lenient, you can squelch them by having
apply.whitespace=nowarn in your configuration file.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
* Not in "next" but will be shortly.
apply.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
114b085dd7b82c3ca74760c896e86c425127cf76
diff --git a/apply.c b/apply.c
index a5cdd8e..d5cb5b1 100644
--- a/apply.c
+++ b/apply.c
@@ -39,7 +39,7 @@ static enum whitespace_eol {
warn_on_whitespace,
error_on_whitespace,
strip_whitespace,
-} new_whitespace = nowarn_whitespace;
+} new_whitespace = warn_on_whitespace;
static int whitespace_error = 0;
static int squelch_whitespace_errors = 5;
static int applied_after_stripping = 0;
@@ -55,6 +55,10 @@ static void parse_whitespace_option(cons
new_whitespace = warn_on_whitespace;
return;
}
+ if (!strcmp(option, "nowarn")) {
+ new_whitespace = nowarn_whitespace;
+ return;
+ }
if (!strcmp(option, "error")) {
new_whitespace = error_on_whitespace;
return;
--
1.2.3.gbfea
^ permalink raw reply related
* [PATCH 2/3] apply --whitespace: configuration option.
From: Junio C Hamano @ 2006-02-28 1:13 UTC (permalink / raw)
To: git; +Cc: Andrew Morton
In-Reply-To: <7vhd6kxuea.fsf@assigned-by-dhcp.cox.net>
The new configuration option apply.whitespace can take one of
"warn", "error", "error-all", or "strip". When git-apply is run
to apply the patch to the index, they are used as the default
value if there is no command line --whitespace option.
Andrew can now tell people who feed him git trees to update to
this version and say:
git repo-config apply.whitespace error
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
* Already in "next".
apply.c | 72 ++++++++++++++++++++++++++++++++++++++-------------------
cache.h | 2 ++
environment.c | 1 +
3 files changed, 51 insertions(+), 24 deletions(-)
2ae1c53b51ff78b13cc8abf8e9798a12140b7638
diff --git a/apply.c b/apply.c
index 8139d83..a5cdd8e 100644
--- a/apply.c
+++ b/apply.c
@@ -35,16 +35,42 @@ static const char apply_usage[] =
"git-apply [--stat] [--numstat] [--summary] [--check] [--index] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [-z] [-pNUM] <patch>...";
static enum whitespace_eol {
- nowarn,
+ nowarn_whitespace,
warn_on_whitespace,
error_on_whitespace,
- strip_and_apply,
-} new_whitespace = nowarn;
+ strip_whitespace,
+} new_whitespace = nowarn_whitespace;
static int whitespace_error = 0;
static int squelch_whitespace_errors = 5;
static int applied_after_stripping = 0;
static const char *patch_input_file = NULL;
+static void parse_whitespace_option(const char *option)
+{
+ if (!option) {
+ new_whitespace = nowarn_whitespace;
+ return;
+ }
+ if (!strcmp(option, "warn")) {
+ new_whitespace = warn_on_whitespace;
+ return;
+ }
+ if (!strcmp(option, "error")) {
+ new_whitespace = error_on_whitespace;
+ return;
+ }
+ if (!strcmp(option, "error-all")) {
+ new_whitespace = error_on_whitespace;
+ squelch_whitespace_errors = 0;
+ return;
+ }
+ if (!strcmp(option, "strip")) {
+ new_whitespace = strip_whitespace;
+ return;
+ }
+ die("unrecognized whitespace option '%s'", option);
+}
+
/*
* For "diff-stat" like behaviour, we keep track of the biggest change
* we've seen, and the longest filename. That allows us to do simple
@@ -832,7 +858,7 @@ static int parse_fragment(char *line, un
* That is, an addition of an empty line would check
* the '+' here. Sneaky...
*/
- if ((new_whitespace != nowarn) &&
+ if ((new_whitespace != nowarn_whitespace) &&
isspace(line[len-2])) {
whitespace_error++;
if (squelch_whitespace_errors &&
@@ -1129,7 +1155,7 @@ static int apply_line(char *output, cons
* patch[plen] is '\n'.
*/
int add_nl_to_tail = 0;
- if ((new_whitespace == strip_and_apply) &&
+ if ((new_whitespace == strip_whitespace) &&
1 < plen && isspace(patch[plen-1])) {
if (patch[plen] == '\n')
add_nl_to_tail = 1;
@@ -1824,10 +1850,21 @@ static int apply_patch(int fd, const cha
return 0;
}
+static int git_apply_config(const char *var, const char *value)
+{
+ if (!strcmp(var, "apply.whitespace")) {
+ apply_default_whitespace = strdup(value);
+ return 0;
+ }
+ return git_default_config(var, value);
+}
+
+
int main(int argc, char **argv)
{
int i;
int read_stdin = 1;
+ const char *whitespace_option = NULL;
for (i = 1; i < argc; i++) {
const char *arg = argv[i];
@@ -1895,30 +1932,17 @@ int main(int argc, char **argv)
continue;
}
if (!strncmp(arg, "--whitespace=", 13)) {
- if (!strcmp(arg+13, "warn")) {
- new_whitespace = warn_on_whitespace;
- continue;
- }
- if (!strcmp(arg+13, "error")) {
- new_whitespace = error_on_whitespace;
- continue;
- }
- if (!strcmp(arg+13, "error-all")) {
- new_whitespace = error_on_whitespace;
- squelch_whitespace_errors = 0;
- continue;
- }
- if (!strcmp(arg+13, "strip")) {
- new_whitespace = strip_and_apply;
- continue;
- }
- die("unrecognized whitespace option '%s'", arg+13);
+ whitespace_option = arg + 13;
+ parse_whitespace_option(arg + 13);
+ continue;
}
if (check_index && prefix_length < 0) {
prefix = setup_git_directory();
prefix_length = prefix ? strlen(prefix) : 0;
- git_config(git_default_config);
+ git_config(git_apply_config);
+ if (!whitespace_option && apply_default_whitespace)
+ parse_whitespace_option(apply_default_whitespace);
}
if (0 < prefix_length)
arg = prefix_filename(prefix, prefix_length, arg);
diff --git a/cache.h b/cache.h
index 58eec00..0d3b244 100644
--- a/cache.h
+++ b/cache.h
@@ -161,11 +161,13 @@ extern int hold_index_file_for_update(st
extern int commit_index_file(struct cache_file *);
extern void rollback_index_file(struct cache_file *);
+/* Environment bits from configuration mechanism */
extern int trust_executable_bit;
extern int assume_unchanged;
extern int only_use_symrefs;
extern int diff_rename_limit_default;
extern int shared_repository;
+extern const char *apply_default_whitespace;
#define GIT_REPO_VERSION 0
extern int repository_format_version;
diff --git a/environment.c b/environment.c
index 251e53c..16c08f0 100644
--- a/environment.c
+++ b/environment.c
@@ -17,6 +17,7 @@ int only_use_symrefs = 0;
int repository_format_version = 0;
char git_commit_encoding[MAX_ENCODING_LENGTH] = "utf-8";
int shared_repository = 0;
+const char *apply_default_whitespace = NULL;
static char *git_dir, *git_object_dir, *git_index_file, *git_refs_dir,
*git_graft_file;
--
1.2.3.gbfea
^ permalink raw reply related
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