* Re: Something is broken in repack
From: Nicolas Pitre @ 2007-12-12 5:12 UTC (permalink / raw)
To: Jon Smirl; +Cc: Junio C Hamano, gcc, Git Mailing List
In-Reply-To: <9e4733910712110821o7748802ag75d9df4be8b2c123@mail.gmail.com>
On Tue, 11 Dec 2007, Jon Smirl wrote:
> On 12/11/07, Nicolas Pitre <nico@cam.org> wrote:
> > On Tue, 11 Dec 2007, Nicolas Pitre wrote:
> >
> > > OK, here's something else for you to try:
> > >
> > > core.deltabasecachelimit=0
> > > pack.threads=2
> > > pack.deltacachesize=1
> > >
> > > With that I'm able to repack the small gcc pack on my machine with 1GB
> > > of ram using:
> > >
> > > git repack -a -f -d --window=250 --depth=250
> > >
> > > and top reports a ~700m virt and ~500m res without hitting swap at all.
> > > It is only at 25% so far, but I was unable to get that far before.
> >
> > Well, around 55% memory usage skyrocketed to 1.6GB and the system went
> > deep into swap. So I restarted it with no threads.
> >
> > Nicolas (even more puzzled)
>
> On the plus side you are seeing what I see, so it proves I am not imagining it.
Well... This is weird.
It seems that memory fragmentation is really really killing us here.
The fact that the Google allocator did manage to waste quite less memory
is a good indicator already.
I did modify the progress display to show accounted memory that was
allocated vs memory that was freed but still not released to the system.
At least that gives you an idea of memory allocation and fragmentation
with glibc in real time:
diff --git a/progress.c b/progress.c
index d19f80c..46ac9ef 100644
--- a/progress.c
+++ b/progress.c
@@ -8,6 +8,7 @@
* published by the Free Software Foundation.
*/
+#include <malloc.h>
#include "git-compat-util.h"
#include "progress.h"
@@ -94,10 +95,12 @@ static int display(struct progress *progress, unsigned n, const char *done)
if (progress->total) {
unsigned percent = n * 100 / progress->total;
if (percent != progress->last_percent || progress_update) {
+ struct mallinfo m = mallinfo();
progress->last_percent = percent;
- fprintf(stderr, "%s: %3u%% (%u/%u)%s%s",
- progress->title, percent, n,
- progress->total, tp, eol);
+ fprintf(stderr, "%s: %3u%% (%u/%u) %u/%uMB%s%s",
+ progress->title, percent, n, progress->total,
+ m.uordblks >> 18, m.fordblks >> 18,
+ tp, eol);
fflush(stderr);
progress_update = 0;
return 1;
This shows that at some point the repack goes into a big memory surge.
I don't have enough RAM to see how fragmented memory gets though, since
it starts swapping around 50% done with 2 threads.
With only 1 thread, memory usage grows significantly at around 11% with
a pretty noticeable slowdown in the progress rate.
So I think the theory goes like this:
There is a block of big objects together in the list somewhere.
Initially, all those big objects are assigned to thread #1 out of 4.
Because those objects are big, they get really slow to delta compress,
and storing them all in a window with 250 slots takes significant
memory.
Threads 2, 3, and 4 have "easy" work loads, so they complete fairly
quicly compared to thread #1. But since the progress display is global
then you won't notice that one thread is actually crawling slowly.
To keep all threads busy until the end, those threads that are done with
their work load will steal some work from another thread, choosing the
one with the largest remaining work. That is most likely thread #1. So
as threads 2, 3, and 4 complete, they will steal from thread 1 and
populate their own window with those big objects too, and get slow too.
And because all threads gets to work on those big objects towards the
end, the progress display will then show a significant slowdown, and
memory usage will almost quadruple.
Add memory fragmentation to that and you have a clogged system.
Solution:
pack.deltacachesize=1
pack.windowmemory=16M
Limiting the window memory to 16MB will automatically shrink the window
size when big objects are encountered, therefore keeping much fewer of
those objects at the same time in memory, which in turn means they will
be processed much more quickly. And somehow that must help with memory
fragmentation as well.
Setting pack.deltacachesize to 1 is simply to disable the caching of
delta results entirely which will only slow down the writing phase, but
I wanted to keep it out of the picture for now.
With the above settings, I'm currently repacking the gcc repo with 2
threads, and memory allocation never exceeded 700m virt and 400m res,
while the mallinfo shows about 350MB, and progress has reached 90% which
has never occurred on this machine with the 300MB source pack so far.
Nicolas
^ permalink raw reply related
* 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 5:13 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: David, Marco Costalba, Andy Parkins, git
In-Reply-To: <20071212041002.GN14735@spearce.org>
On Dec 11, 2007 11:10 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Jason Sewall <jasonsewall@gmail.com> wrote:
> > Anyway, ugit is very good for a first draft; its text display beats
> > whats in git-gui in a big way (and I would *hope* qt4 would beat
> > Tcl/Tk at that at least).
>
> Are you just using the wrong fonts under git-gui? I mean both
> Tk and qt4 are drawing text through your windowing system, from
> the same pool of font files... if qt4 can draw nice text then
> so can Tk, right?
I don't know much about graphical toolkits and the like, but I think
that the more modern ones have fancy features like antialiasing and
subpixel rendering, which makes a big difference when you're working
on a laptop with a tiny screen.
Take a look for yourself:
http://img441.imageshack.us/img441/492/comparejd6.png
They are obviously using different fonts there (because I can't figure
out what font ugit is using) but there is a difference in rendering
quality to be sure.
The qt stuff fits better with the rest of my system better too (even
though I'm using gnome) - it's entirely the result of Tk being
lightweight and a million years old, when UI conventions were
different (like every menu being detachable, and antique scrollbars).
I'm not here to start a toolkit flame war (we had a toolkit dogpile on
the list last week, I think) I'm just pointing out that Tk is from a
different era.
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.
Jason
^ 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: Shawn O. Pearce @ 2007-12-12 5:23 UTC (permalink / raw)
To: Jason Sewall; +Cc: David, Marco Costalba, Andy Parkins, git
In-Reply-To: <31e9dd080712112113u44b30c62ja012951fba958c5d@mail.gmail.com>
Jason Sewall <jasonsewall@gmail.com> wrote:
> I don't know much about graphical toolkits and the like, but I think
> that the more modern ones have fancy features like antialiasing and
> subpixel rendering, which makes a big difference when you're working
> on a laptop with a tiny screen.
Oh, that's a good point. On my Mac OS X system with the aqua port
of Tk the fonts render just as good as anything else on this box.
I guess the Aqua port of Tk is just better than the X11 port of
Tk is. :)
> Take a look for yourself:
> http://img441.imageshack.us/img441/492/comparejd6.png
>
> They are obviously using different fonts there (because I can't figure
> out what font ugit is using) but there is a difference in rendering
> quality to be sure.
Be nice to know what ugit is using, or really how its guessing the
default font. I wonder what font you are using with your git-gui.
The default Tk picks on X11 is basically crap, but git-gui goes
with your system default as its own default.
> The qt stuff fits better with the rest of my system better too (even
> though I'm using gnome) - it's entirely the result of Tk being
> lightweight and a million years old, when UI conventions were
> different (like every menu being detachable, and antique scrollbars).
> I'm not here to start a toolkit flame war (we had a toolkit dogpile on
> the list last week, I think) I'm just pointing out that Tk is from a
> different era.
Yes. The tile extension in 8.5 should actually improve this quite
a bit; as I understand it there is a GTK backend for Tk with that
set of extensions, making the UI look more modern on X11, assuming
GTK was available when Tk was compiled, etc...
I have yet to make git-gui use the tile extension. Its however
planned to happen in the near-ish future.
> 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.
--
Shawn.
^ permalink raw reply
* [PATCH 1/2] git-help: add "help.format" config variable.
From: Christian Couder @ 2007-12-12 5:33 UTC (permalink / raw)
To: Junio Hamano; +Cc: git
This config variable makes it possible to choose the default format
used to display help. This format will be used only if no option
like -a|--all|-i|--info|-m|--man|-w|--web is passed to "git-help".
The following values are possible for this variable:
- "man" --> "man" program is used
- "info" --> "info" program is used
- "web" --> "git-browse-help" is used
By default we still show help using "man".
By the way, this patch also adds -m|--man command line option to
use "man" even if something else is set in the "help.format"
config variable.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-help.txt | 13 ++++++++-
git.c | 2 +-
help.c | 61 ++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 72 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index ac9e15d..af5e5cb 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -7,7 +7,7 @@ git-help - display help information about git
SYNOPSIS
--------
-'git help' [-a|--all|-i|--info|-w|--web] [COMMAND]
+'git help' [-a|--all|-i|--info|-m|--man|-w|--web] [COMMAND]
DESCRIPTION
-----------
@@ -23,6 +23,12 @@ If a git command is named, a manual page for that command is brought
up. The 'man' program is used by default for this purpose, but this
can be overriden by other options.
+If no command line option is passed, the 'help.format' configuration
+variable will be checked. Supported values for this variable are
+"man", "info" and "web" (or "html" as a synonym to the latter). This
+makes git-help behave as if a command line option with the same long
+name as been passed to it.
+
Note that 'git --help ...' is identical as 'git help ...' because the
former is internally converted into the latter.
@@ -36,6 +42,11 @@ OPTIONS
Use the 'info' program to display the manual page, instead of
the 'man' program that is used by default.
+-m|--man::
+ Use the 'man' program to display the manual page. This may be
+ used to override a value set in the 'help.format'
+ configuration variable.
+
-w|--web::
Use a web browser to display the HTML manual page, instead of
the 'man' program that is used by default.
diff --git a/git.c b/git.c
index 4f9876e..d46b63d 100644
--- a/git.c
+++ b/git.c
@@ -324,7 +324,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "gc", cmd_gc, RUN_SETUP },
{ "get-tar-commit-id", cmd_get_tar_commit_id },
{ "grep", cmd_grep, RUN_SETUP | USE_PAGER },
- { "help", cmd_help },
+ { "help", cmd_help, RUN_SETUP },
#ifndef NO_CURL
{ "http-fetch", cmd_http_fetch, RUN_SETUP },
#endif
diff --git a/help.c b/help.c
index c96b167..af0a433 100644
--- a/help.c
+++ b/help.c
@@ -8,6 +8,44 @@
#include "exec_cmd.h"
#include "common-cmds.h"
+static const char *help_default_format;
+
+static enum help_format {
+ man_format,
+ info_format,
+ web_format,
+} help_format = man_format;
+
+static void parse_help_format(const char *format)
+{
+ if (!format) {
+ help_format = man_format;
+ return;
+ }
+ if (!strcmp(format, "man")) {
+ help_format = man_format;
+ return;
+ }
+ if (!strcmp(format, "info")) {
+ help_format = info_format;
+ return;
+ }
+ if (!strcmp(format, "web") || !strcmp(format, "html")) {
+ help_format = web_format;
+ return;
+ }
+ die("unrecognized help format '%s'", format);
+}
+
+static int git_help_config(const char *var, const char *value)
+{
+ if (!strcmp(var, "help.format")) {
+ help_default_format = xstrdup(value);
+ return 0;
+ }
+ return git_default_config(var, value);
+}
+
/* most GUI terminals set COLUMNS (although some don't export it) */
static int term_columns(void)
{
@@ -331,8 +369,27 @@ int cmd_help(int argc, const char **argv, const char *prefix)
show_info_page(argc > 2 ? argv[2] : NULL);
}
- else
- show_man_page(help_cmd);
+ else if (!strcmp(help_cmd, "--man") || !strcmp(help_cmd, "-m")) {
+ show_man_page(argc > 2 ? argv[2] : NULL);
+ }
+
+ else {
+ git_config(git_help_config);
+ if (help_default_format)
+ parse_help_format(help_default_format);
+
+ switch (help_format) {
+ case man_format:
+ show_man_page(help_cmd);
+ break;
+ case info_format:
+ show_info_page(help_cmd);
+ break;
+ case web_format:
+ show_html_page(help_cmd);
+ break;
+ }
+ }
return 0;
}
--
1.5.3.7.2270.g786cf-dirty
^ permalink raw reply related
* [PATCH 2/2] git-help: add "web_or_man" and "web_or_info" config vars.
From: Christian Couder @ 2007-12-12 5:33 UTC (permalink / raw)
To: Junio Hamano; +Cc: git
These configuration variables can be used when you want to
use a web browser if DISPLAY is set and "man" or "info"
otherwise.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-help.txt | 15 +++++++++++----
help.c | 10 ++++++++++
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index af5e5cb..c790584 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -24,10 +24,17 @@ up. The 'man' program is used by default for this purpose, but this
can be overriden by other options.
If no command line option is passed, the 'help.format' configuration
-variable will be checked. Supported values for this variable are
-"man", "info" and "web" (or "html" as a synonym to the latter). This
-makes git-help behave as if a command line option with the same long
-name as been passed to it.
+variable will be checked. The following values are supported for this
+variable; they make 'git-help' behave as their corresponding command
+line option:
+
+* "man" corresponds to '-m|--man',
+* "info" corresponds to '-i|--info',
+* "web" or "html" correspond to '-w|--web',
+* "web_or_man" corresponds to '-w|--web' if DISPLAY is set, '-m|--man'
+otherwise,
+* "web_or_info" corresponds to '-w|--web' if DISPLAY is set, '-i|--info'
+otherwise.
Note that 'git --help ...' is identical as 'git help ...' because the
former is internally converted into the latter.
diff --git a/help.c b/help.c
index af0a433..d46bc08 100644
--- a/help.c
+++ b/help.c
@@ -34,6 +34,16 @@ static void parse_help_format(const char *format)
help_format = web_format;
return;
}
+ if (!strcmp(format, "web_or_man")) {
+ char *display = getenv("DISPLAY");
+ help_format = display ? web_format : man_format;
+ return;
+ }
+ if (!strcmp(format, "web_or_info")) {
+ char *display = getenv("DISPLAY");
+ help_format = display ? web_format : info_format;
+ return;
+ }
die("unrecognized help format '%s'", format);
}
--
1.5.3.7.2269.g4ae7
^ permalink raw reply related
* Re: [PATCH 2/2] git-help: add "web_or_man" and "web_or_info" config vars.
From: Junio C Hamano @ 2007-12-12 5:35 UTC (permalink / raw)
To: Christian Couder; +Cc: git
In-Reply-To: <20071212063325.92cd29a8.chriscool@tuxfamily.org>
I personally feel this is going a bit overboard. We'd be better off
doing something like this:
- add 'help.external' configuration variable, that allows the user to
set any external program;
- spawn the external program when "git help <cmd>" is given, with <cmd>
as the parameter;
- add an option to 'git config' to dump various paths configured in the
git program, such as manual page path, info page path and such, so
that the external program can learn where things are.
The second point above means that translating "cat-file" to
"git-cat-file.html" becomes the responsibility for the external
program.
^ permalink raw reply
* Re: Using git with Eclipse
From: Wink Saville @ 2007-12-12 6:07 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: Shawn O. Pearce, git
In-Reply-To: <200712120129.43878.robin.rosenberg.lists@dewire.com>
Robin Rosenberg wrote:
> tisdag 11 december 2007 skrev Wink Saville:
>
> <snip>
> -- robin
>
I case I'll take the bait and a guinea pig:)
Please send me a link to what you'd like me to test.
Thanks,
Wink
^ permalink raw reply
* exit with error for "git remote update"
From: Robert Schiele @ 2007-12-12 6:17 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 943 bytes --]
Hi,
When I call "git remote update" without an additional parameter it always
returns with an error, even when all fetch commands processed without a
problem.
This is because of the exit(1) in the following code:
elsif ($ARGV[0] eq 'update') {
if (@ARGV <= 1) {
update_remote("default");
exit(1);
}
for ($i = 1; $i < @ARGV; $i++) {
update_remote($ARGV[$i]);
}
}
Shouldn't this exit(1) be an exit(0) instead or just rewrite this part of the
code as
elsif ($ARGV[0] eq 'update') {
if (@ARGV <= 1) {
update_remote("default");
} else {
for ($i = 1; $i < @ARGV; $i++) {
update_remote($ARGV[$i]);
}
}
}
Robert
--
Robert Schiele
Dipl.-Wirtsch.informatiker mailto:rschiele@gmail.com
"Quidquid latine dictum sit, altum sonatur."
[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]
^ permalink raw reply
* Re: [(not so) random thoughts] using git as its own caching tool
From: Mike Hommey @ 2007-12-12 6:51 UTC (permalink / raw)
To: Pierre Habouzit, Git ML
In-Reply-To: <20071212003813.GG29110@artemis.madism.org>
On Wed, Dec 12, 2007 at 01:38:13AM +0100, 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 ?
I love this idea.
Mike
^ permalink raw reply
* Re: [PATCH 2/2] git-help: add "web_or_man" and "web_or_info" config vars.
From: Christian Couder @ 2007-12-12 7:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v1w9sbhlg.fsf@gitster.siamese.dyndns.org>
Le mercredi 12 décembre 2007, Junio C Hamano a écrit :
> I personally feel this is going a bit overboard.
The main problem without it is for people who like web browser, but often
use ssh without -Y. In this latter case, they may enter into "manly" mode,
you know, and don't want to be bothered to install or use some kind of text
based web browser.
Christian.
^ permalink raw reply
* git show stash
From: Mike Hommey @ 2007-12-12 7:28 UTC (permalink / raw)
To: git
Hi,
Is there a particular reason why git show stash displays 2 signs at the
beginning of changed lines, contrary to git show anyotherref that shows
only one ? (the diff header is also different)
Mike
^ permalink raw reply
* Re: git show stash
From: Mike Hommey @ 2007-12-12 7:37 UTC (permalink / raw)
To: git
In-Reply-To: <20071212072836.GA10961@glandium.org>
On Wed, Dec 12, 2007 at 08:28:36AM +0100, Mike Hommey wrote:
> Hi,
>
> Is there a particular reason why git show stash displays 2 signs at the
> beginning of changed lines, contrary to git show anyotherref that shows
> only one ? (the diff header is also different)
I got a hint by opening my eyes: the stash is a merge commit (why is
that so, by the way ?).
Mike
^ permalink raw reply
* Re: [PATCH (amend)] diff: Make numstat machine friendly also for renames (and copies)
From: Junio C Hamano @ 2007-12-12 7:46 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <200712120009.36560.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> The previous patch is a fix (usability fix) for numstat output in the
> presence of rename detection, making it truly machine friendly. Moreover
> it should not break any scripts which parsed numstat output not
> expecting to deal with renames (for example if repository has
> diff.renames config option set), and might even fix them.
>
> The proposed - and implemented in patch below - change could break some
> scripts not expecting to deal with new numstat output.
I do not quite follow. If a script wanted -M and/or -C output in the
older "common/{a => b}/common" format, either change would break it
equally badly, either with or without -z. And I do not think adding
status letter is necessary for --numstat; if the caller wants that
information, it can ask for it explicitly with both options.
So I do not think there is anything more to solve, than the patch you
just sent.
The patch looks good. Thanks.
> P.S. The numstat output format for renames should be probably described
> in documentation, and not only in commit message, but I was not sure
> where to put it (and even how it should be written).
I started writing this, and then reviewed the result of squashing your
two patches.
Although everybody complains that most of git is run-once-and-exit, I
wrote the original diff part fairly conservatively to make it leak-free,
because of a single command, "diff-tree --stdin", that can be told to
run millions of diffs inside a single process.
But the diffstat part is horribly leaky, especially after your patch,
and it has ugly workarounds such as refusing to show both diffstat and
shortstat at the same time, not because it does not make much sense
(admittedly it doesn't), but because show_stats() discards necessary
information when it is done, making it impossible for shortstat to run.
So I ended up restructuring the name allocation/free code a bit while at
it.
-- >8 --
diff --numstat -z: make it machine readable
The "-z" format is all about machine parsability, but showing renamed
paths as "common/{a => b}/suffix" makes it impossible. The scripts would
never have successfully parsed "--numstat -z -M" in the old format.
This fixes the output format in a (hopefully minimally) backward
incompatible way.
* The output without -z is not changed. This has given a good way for
humans to view added and deleted lines separately, and showing the
path in combined, shorter way would preserve readability.
* The output with -z is unchanged for paths that do not involve renames.
Existing scripts that do not pass -M/-C are not affected at all.
* The output with -z for a renamed path is shown in a format that can
easily be distinguished from an unrenamed path.
This is based on Jakub Narebski's patch. Bugs and documentation typos
are mine.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/diff-format.txt | 61 +++++++++++++++++++++++++
diff.c | 100 ++++++++++++++++++++++++++++-------------
2 files changed, 129 insertions(+), 32 deletions(-)
diff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt
index 2c3a4c4..400cbb3 100644
--- a/Documentation/diff-format.txt
+++ b/Documentation/diff-format.txt
@@ -84,3 +84,64 @@ all parents.
include::diff-generate-patch.txt[]
+
+
+other diff formats
+------------------
+
+The `--summary` option describes newly added, deleted, renamed and
+copied files. The `--stat` option adds diffstat(1) graph to the
+output. These options can be combined with other options, such as
+`-p`, and are meant for human consumption.
+
+When showing a change that involves a rename or a copy, `--stat` output
+formats the pathnames compactly by combining common prefix and suffix of
+the pathnames. For example, a change that moves `arch/i386/Makefile` to
+`arch/x86/Makefile` while modifying 4 lines will be shown like this:
+
+------------------------------------
+arch/{i386 => x86}/Makefile | 4 +--
+------------------------------------
+
+The `--numstat` option gives the diffstat(1) information but is designed
+for easier machine consumption. An entry in `--numstat` output looks
+like this:
+
+----------------------------------------
+1 2 README
+3 1 arch/{i386 => x86}/Makefile
+----------------------------------------
+
+That is, from left to right:
+
+. the number of added lines;
+. a tab;
+. the number of deleted lines;
+. a tab;
+. pathname (possibly with rename/copy information);
+. a newline.
+
+When `-z` output option is in effect, the output is formatted this way:
+
+----------------------------------------
+1 2 README NUL
+3 1 NUL arch/i386/Makefile NUL arch/x86/Makefile NUL
+----------------------------------------
+
+That is:
+
+. the number of added lines;
+. a tab;
+. the number of deleted lines;
+. a tab;
+. a NUL (only exists if renamed/copied);
+. pathname in preimage;
+. a NUL (only exists if renamed/copied);
+. pathname in postimage (only exists if renamed/copied);
+. a NUL.
+
+The extra `NUL` before the preimage path in renamed case is to allow
+scripts that read the output to tell if the current record being read is
+a single-path record or a rename/copy record without reading ahead.
+After reading added and deleted lines, reading up to `NUL` would yield
+the pathname, but if that is `NUL`, the record will show two paths.
diff --git a/diff.c b/diff.c
index f780e3e..fc51506 100644
--- a/diff.c
+++ b/diff.c
@@ -734,7 +734,9 @@ struct diffstat_t {
int nr;
int alloc;
struct diffstat_file {
+ char *from_name;
char *name;
+ char *print_name;
unsigned is_unmerged:1;
unsigned is_binary:1;
unsigned is_renamed:1;
@@ -755,11 +757,14 @@ static struct diffstat_file *diffstat_add(struct diffstat_t *diffstat,
}
diffstat->files[diffstat->nr++] = x;
if (name_b) {
- x->name = pprint_rename(name_a, name_b);
+ x->from_name = xstrdup(name_a);
+ x->name = xstrdup(name_b);
x->is_renamed = 1;
}
- else
+ else {
+ x->from_name = NULL;
x->name = xstrdup(name_a);
+ }
return x;
}
@@ -803,6 +808,28 @@ static void show_graph(char ch, int cnt, const char *set, const char *reset)
printf("%s", reset);
}
+static void fill_print_name(struct diffstat_file *file)
+{
+ char *pname;
+
+ if (file->print_name)
+ return;
+
+ if (!file->is_renamed) {
+ struct strbuf buf;
+ strbuf_init(&buf, 0);
+ if (quote_c_style(file->name, &buf, NULL, 0)) {
+ pname = strbuf_detach(&buf, NULL);
+ } else {
+ pname = file->name;
+ strbuf_release(&buf);
+ }
+ } else {
+ pname = pprint_rename(file->from_name, file->name);
+ }
+ file->print_name = pname;
+}
+
static void show_stats(struct diffstat_t* data, struct diff_options *options)
{
int i, len, add, del, total, adds = 0, dels = 0;
@@ -836,19 +863,8 @@ static void show_stats(struct diffstat_t* data, struct diff_options *options)
for (i = 0; i < data->nr; i++) {
struct diffstat_file *file = data->files[i];
int change = file->added + file->deleted;
-
- if (!file->is_renamed) { /* renames are already quoted by pprint_rename */
- struct strbuf buf;
- strbuf_init(&buf, 0);
- if (quote_c_style(file->name, &buf, NULL, 0)) {
- free(file->name);
- file->name = strbuf_detach(&buf, NULL);
- } else {
- strbuf_release(&buf);
- }
- }
-
- len = strlen(file->name);
+ fill_print_name(file);
+ len = strlen(file->print_name);
if (max_len < len)
max_len = len;
@@ -873,7 +889,7 @@ static void show_stats(struct diffstat_t* data, struct diff_options *options)
for (i = 0; i < data->nr; i++) {
const char *prefix = "";
- char *name = data->files[i]->name;
+ char *name = data->files[i]->print_name;
int added = data->files[i]->added;
int deleted = data->files[i]->deleted;
int name_len;
@@ -901,17 +917,17 @@ static void show_stats(struct diffstat_t* data, struct diff_options *options)
printf("%s%d%s", add_c, added, reset);
printf(" bytes");
printf("\n");
- goto free_diffstat_file;
+ continue;
}
else if (data->files[i]->is_unmerged) {
show_name(prefix, name, len, reset, set);
printf(" Unmerged\n");
- goto free_diffstat_file;
+ continue;
}
else if (!data->files[i]->is_renamed &&
(added + deleted == 0)) {
total_files--;
- goto free_diffstat_file;
+ continue;
}
/*
@@ -933,11 +949,7 @@ static void show_stats(struct diffstat_t* data, struct diff_options *options)
show_graph('+', add, add_c, reset);
show_graph('-', del, del_c, reset);
putchar('\n');
- free_diffstat_file:
- free(data->files[i]->name);
- free(data->files[i]);
}
- free(data->files);
printf("%s %d files changed, %d insertions(+), %d deletions(-)%s\n",
set, total_files, adds, dels, reset);
}
@@ -962,11 +974,7 @@ static void show_shortstats(struct diffstat_t* data)
dels += deleted;
}
}
- free(data->files[i]->name);
- free(data->files[i]);
}
- free(data->files);
-
printf(" %d files changed, %d insertions(+), %d deletions(-)\n",
total_files, adds, dels);
}
@@ -975,6 +983,9 @@ static void show_numstat(struct diffstat_t* data, struct diff_options *options)
{
int i;
+ if (data->nr == 0)
+ return;
+
for (i = 0; i < data->nr; i++) {
struct diffstat_file *file = data->files[i];
@@ -982,15 +993,39 @@ static void show_numstat(struct diffstat_t* data, struct diff_options *options)
printf("-\t-\t");
else
printf("%d\t%d\t", file->added, file->deleted);
- if (!file->is_renamed) {
- write_name_quoted(file->name, stdout, options->line_termination);
+ if (options->line_termination) {
+ fill_print_name(file);
+ if (!file->is_renamed)
+ write_name_quoted(file->name, stdout,
+ options->line_termination);
+ else {
+ fputs(file->print_name, stdout);
+ putchar(options->line_termination);
+ }
} else {
- fputs(file->name, stdout);
- putchar(options->line_termination);
+ if (file->is_renamed) {
+ putchar('\0');
+ write_name_quoted(file->from_name, stdout, '\0');
+ }
+ write_name_quoted(file->name, stdout, '\0');
}
}
}
+static void free_diffstat_info(struct diffstat_t *diffstat)
+{
+ int i;
+ for (i = 0; i < diffstat->nr; i++) {
+ struct diffstat_file *f = diffstat->files[i];
+ if (f->name != f->print_name)
+ free(f->print_name);
+ free(f->name);
+ free(f->from_name);
+ free(f);
+ }
+ free(diffstat->files);
+}
+
struct checkdiff_t {
struct xdiff_emit_state xm;
const char *filename;
@@ -2943,8 +2978,9 @@ void diff_flush(struct diff_options *options)
show_numstat(&diffstat, options);
if (output_format & DIFF_FORMAT_DIFFSTAT)
show_stats(&diffstat, options);
- else if (output_format & DIFF_FORMAT_SHORTSTAT)
+ if (output_format & DIFF_FORMAT_SHORTSTAT)
show_shortstats(&diffstat);
+ free_diffstat_info(&diffstat);
separator++;
}
^ permalink raw reply related
* Re: git annotate runs out of memory
From: Jeff King @ 2007-12-12 7:57 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Daniel Berlin, Git Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712111146200.25032@woody.linux-foundation.org>
On Tue, Dec 11, 2007 at 11:50:08AM -0800, Linus Torvalds wrote:
> And, btw: the diff is totally different from the xdelta we have, so even
> if we have an already prepared nice xdelta between the two versions, we'll
> end up re-generating the files in full, and then do a diff on the end
> result.
>
> Of course, part of that is that git logically *never* works with deltas,
> except in the actual code-paths that generate objects (or generate packs,
> of course). So even if we had used a delta algorithm that would be
> amenable to be turned into a diff directly, it would have been a layering
> violation to actually do that.
That doesn't mean we can't opportunistically jump layers when available,
and fall back on the regular behavior otherwise. The nice thing about
clean and simple layers is that you can always add optimizations later
by poking sane holes.
Let's assume for the sake of argument that we can convert an xdelta into
a diff fairly cheaply. Using the patch below, we can count the places
where we are diffing two blobs, and one blob is a delta base of the
other (assuming our magical conversion function can also reverse diffs.
;) ).
For a "git log -p" on git.git, I get:
9951 diffs could be optimized
10958 diffs could not be optimized
or about 48%. It would be nice if we could drop the cost by almost 50%
(if our magical function is free to call, too!).
Of course, I haven't even looked at whether converting xdeltas to
unified diffs is possible. I suspect in some cases it is (e.g., pure
addition of text) and in some cases it isn't (I assume xdelta doesn't
have any context lines, which might hurt). And it's possible that a
specialized diff user like git-blame can just learn to use the xdeltas
by itself (I didn't get a "could optimize" count for git-blame since
it seems to follow a different codepath for its diffs).
---
diff --git a/cache.h b/cache.h
index 27d90fe..0d672be 100644
--- a/cache.h
+++ b/cache.h
@@ -569,6 +569,7 @@ extern void *unpack_entry(struct packed_git *, off_t, enum object_type *, unsign
extern unsigned long unpack_object_header_gently(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
extern unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);
extern const char *packed_object_info_detail(struct packed_git *, off_t, unsigned long *, unsigned long *, unsigned int *, unsigned char *);
+extern int have_xdelta(unsigned char from[20], unsigned char to[20]);
extern int matches_pack_name(struct packed_git *p, const char *name);
/* Dumb servers support */
diff --git a/diff.c b/diff.c
index f780e3e..5402900 100644
--- a/diff.c
+++ b/diff.c
@@ -1299,6 +1299,10 @@ static void builtin_diff(const char *name_a,
}
}
+ fprintf(stderr, "could optimize: %s\n",
+ (have_xdelta(one->sha1, two->sha1) ||
+ have_xdelta(two->sha1, one->sha1)) ? "yes" : "no");
+
if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
die("unable to read files to diff");
diff --git a/sha1_file.c b/sha1_file.c
index b0c2435..f811ddc 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2422,3 +2422,20 @@ int read_pack_header(int fd, struct pack_header *header)
return PH_ERROR_PROTOCOL;
return 0;
}
+
+int have_xdelta(unsigned char from[20], unsigned char to[20])
+{
+ struct pack_entry e;
+ unsigned char base_sha1[20];
+ const char *type;
+ unsigned long size;
+ unsigned long store_size;
+ unsigned int delta_chain_length;
+
+ if (!find_pack_entry(to, &e, NULL))
+ return 0;
+
+ type = packed_object_info_detail(e.p, e.offset, &size, &store_size,
+ &delta_chain_length, base_sha1);
+ return !hashcmp(base_sha1, from);
+}
^ permalink raw reply related
* Re: git show stash
From: しらいしななこ @ 2007-12-12 8:02 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
In-Reply-To: <20071212073728.GA12058@glandium.org>
Quoting Mike Hommey <mh@glandium.org>:
> On Wed, Dec 12, 2007 at 08:28:36AM +0100, Mike Hommey wrote:
>> Hi,
>>
>> Is there a particular reason why git show stash displays 2 signs at the
>> beginning of changed lines, contrary to git show anyotherref that shows
>> only one ? (the diff header is also different)
>
> I got a hint by opening my eyes: the stash is a merge commit (why is
> that so, by the way ?).
Because it needs to record changes between your HEAD and your index, and changes between your HEAD and your working files.
.-----STASH
/ /
HEAD ---INDEX
A stash is a merge between your HEAD (first parent) and a commit that records your index state (second parent). The latter commit is a child of your HEAD. If you never run "git add" and always commit with "git commit -a", it is most likely that "git stash show -p" will show two plus signs or minus signs and nothing else, but if you use "git add" to add your changes incrementally to your index you will see the difference.
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
----------------------------------------------------------------------
Get a free email address with REAL anti-spam protection.
http://www.bluebottle.com/tag/1
^ permalink raw reply
* Re: Something is broken in repack
From: David Kastrup @ 2007-12-12 8:05 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Jon Smirl, Junio C Hamano, gcc, Git Mailing List
In-Reply-To: <alpine.LFD.0.99999.0712112057390.555@xanadu.home>
Nicolas Pitre <nico@cam.org> writes:
> Well... This is weird.
>
> It seems that memory fragmentation is really really killing us here.
> The fact that the Google allocator did manage to waste quite less memory
> is a good indicator already.
Maybe an malloc/free/mmap wrapper that records the requested sizes and
alloc/free order and dumps them to file so that one can make a compact
git-free standalone test case for the glibc maintainers might be a good
thing.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* [PATCH 2/2] "diff --check" should affect exit status
From: Wincent Colaiuta @ 2007-12-12 8:12 UTC (permalink / raw)
To: git; +Cc: gitster, Wincent Colaiuta
In-Reply-To: <1197447122-93416-2-git-send-email-win@wincent.com>
"git diff" has a --check option that can be used to check for whitespace
problems but it only reported by printing warnings to the
console.
Now when the --check option is used we give a non-zero exit status,
making "git diff --check" nicer to use in scripts and hooks.
Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
Documentation/diff-options.txt | 4 +++-
builtin-diff-files.c | 2 ++
builtin-diff-index.c | 2 ++
builtin-diff-tree.c | 2 ++
builtin-diff.c | 3 ++-
diff.c | 37 ++++++++++++++++++++++++-------------
diff.h | 1 +
t/t4015-diff-whitespace.sh | 34 ++++++++++++++++++++++++++++++++++
8 files changed, 70 insertions(+), 15 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 5d22b7b..9ecc1d7 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -93,7 +93,9 @@ endif::git-format-patch[]
--check::
Warn if changes introduce trailing whitespace
- or an indent that uses a space before a tab.
+ or an indent that uses a space before a tab. Exits with
+ non-zero status if problems are found. Not compatible with
+ --exit-code.
--full-index::
Instead of the first handful characters, show full
diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 046b7e3..4afc872 100644
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c
@@ -33,5 +33,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
result = run_diff_files_cmd(&rev, argc, argv);
if (DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
return DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0;
+ if (rev.diffopt.output_format & DIFF_FORMAT_CHECKDIFF)
+ return DIFF_OPT_TST(&rev.diffopt, CHECK_FAILED) != 0;
return result;
}
diff --git a/builtin-diff-index.c b/builtin-diff-index.c
index 556c506..532b284 100644
--- a/builtin-diff-index.c
+++ b/builtin-diff-index.c
@@ -46,5 +46,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
result = run_diff_index(&rev, cached);
if (DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
return DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0;
+ if (rev.diffopt.output_format & DIFF_FORMAT_CHECKDIFF)
+ return DIFF_OPT_TST(&rev.diffopt, CHECK_FAILED) != 0;
return result;
}
diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index 2e13716..556c31d 100644
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -134,6 +134,8 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
else
diff_tree_stdin(line);
}
+ if (opt->diffopt.output_format & DIFF_FORMAT_CHECKDIFF)
+ return DIFF_OPT_TST(&opt->diffopt, CHECK_FAILED) != 0;
return DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS)
&& DIFF_OPT_TST(&opt->diffopt, HAS_CHANGES);
}
diff --git a/builtin-diff.c b/builtin-diff.c
index 1b61599..6aeea93 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -365,7 +365,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
ent, ents);
if (DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
result = DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0;
-
+ if (rev.diffopt.output_format & DIFF_FORMAT_CHECKDIFF)
+ return DIFF_OPT_TST(&rev.diffopt, CHECK_FAILED) != 0;
if (1 < rev.diffopt.skip_stat_unmatch)
refresh_index_quietly();
return result;
diff --git a/diff.c b/diff.c
index e74a303..e89c7ce 100644
--- a/diff.c
+++ b/diff.c
@@ -996,6 +996,7 @@ struct checkdiff_t {
const char *filename;
int lineno, color_diff;
unsigned ws_rule;
+ int status;
};
static void checkdiff_consume(void *priv, char *line, unsigned long len)
@@ -1022,6 +1023,7 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
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");
@@ -1412,7 +1414,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
diff_free_filespec_data(two);
}
-static void builtin_checkdiff(const char *name_a, const char *name_b,
+static int builtin_checkdiff(const char *name_a, const char *name_b,
struct diff_filespec *one,
struct diff_filespec *two, struct diff_options *o)
{
@@ -1420,7 +1422,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
struct checkdiff_t data;
if (!two)
- return;
+ return 0;
memset(&data, 0, sizeof(data));
data.xm.consume = checkdiff_consume;
@@ -1449,6 +1451,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
free_and_return:
diff_free_filespec_data(one);
diff_free_filespec_data(two);
+ return data.status;
}
struct diff_filespec *alloc_filespec(const char *path)
@@ -2033,14 +2036,14 @@ static void run_diffstat(struct diff_filepair *p, struct diff_options *o,
builtin_diffstat(name, other, p->one, p->two, diffstat, o, complete_rewrite);
}
-static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
+static int run_checkdiff(struct diff_filepair *p, struct diff_options *o)
{
const char *name;
const char *other;
if (DIFF_PAIR_UNMERGED(p)) {
/* unmerged */
- return;
+ return 0;
}
name = p->one->path;
@@ -2049,7 +2052,7 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
diff_fill_sha1_info(p->one);
diff_fill_sha1_info(p->two);
- builtin_checkdiff(name, other, p->one, p->two, o);
+ return builtin_checkdiff(name, other, p->one, p->two, o);
}
void diff_setup(struct diff_options *options)
@@ -2079,7 +2082,12 @@ int diff_setup_done(struct diff_options *options)
if (options->output_format & DIFF_FORMAT_NAME_STATUS)
count++;
if (options->output_format & DIFF_FORMAT_CHECKDIFF)
+ {
count++;
+ if (DIFF_OPT_TST(options, QUIET) ||
+ DIFF_OPT_TST(options, EXIT_WITH_STATUS))
+ die("--check may not be used with --quiet or --exit-code");
+ }
if (options->output_format & DIFF_FORMAT_NO_OUTPUT)
count++;
if (count > 1)
@@ -2546,17 +2554,17 @@ static void diff_flush_stat(struct diff_filepair *p, struct diff_options *o,
run_diffstat(p, o, diffstat);
}
-static void diff_flush_checkdiff(struct diff_filepair *p,
+static int diff_flush_checkdiff(struct diff_filepair *p,
struct diff_options *o)
{
if (diff_unmodified_pair(p))
- return;
+ return 0;
if ((DIFF_FILE_VALID(p->one) && S_ISDIR(p->one->mode)) ||
(DIFF_FILE_VALID(p->two) && S_ISDIR(p->two->mode)))
- return; /* no tree diffs in patch format */
+ return 0; /* no tree diffs in patch format */
- run_checkdiff(p, o);
+ return run_checkdiff(p, o);
}
int diff_queue_is_empty(void)
@@ -2675,16 +2683,19 @@ static int check_pair_status(struct diff_filepair *p)
}
}
-static void flush_one_pair(struct diff_filepair *p, struct diff_options *opt)
+static int flush_one_pair(struct diff_filepair *p, struct diff_options *opt)
{
int fmt = opt->output_format;
if (fmt & DIFF_FORMAT_CHECKDIFF)
- diff_flush_checkdiff(p, opt);
+ return diff_flush_checkdiff(p, opt);
else if (fmt & (DIFF_FORMAT_RAW | DIFF_FORMAT_NAME_STATUS))
diff_flush_raw(p, opt);
else if (fmt & DIFF_FORMAT_NAME)
write_name_quoted(p->two->path, stdout, opt->line_termination);
+
+ /* return value only matters with DIFF_FORMAT_CHECKDIFF */
+ return 0;
}
static void show_file_mode_name(const char *newdelete, struct diff_filespec *fs)
@@ -2923,8 +2934,8 @@ void diff_flush(struct diff_options *options)
DIFF_FORMAT_CHECKDIFF)) {
for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i];
- if (check_pair_status(p))
- flush_one_pair(p, options);
+ if (check_pair_status(p) && flush_one_pair(p, options))
+ DIFF_OPT_SET(options, CHECK_FAILED);
}
separator++;
}
diff --git a/diff.h b/diff.h
index a52496a..5d50d93 100644
--- a/diff.h
+++ b/diff.h
@@ -59,6 +59,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
#define DIFF_OPT_ALLOW_EXTERNAL (1 << 13)
#define DIFF_OPT_EXIT_WITH_STATUS (1 << 14)
#define DIFF_OPT_REVERSE_DIFF (1 << 15)
+#define DIFF_OPT_CHECK_FAILED (1 << 16)
#define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag)
#define DIFF_OPT_SET(opts, flag) ((opts)->flags |= DIFF_OPT_##flag)
#define DIFF_OPT_CLR(opts, flag) ((opts)->flags &= ~DIFF_OPT_##flag)
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 79fdff3..c110201 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -117,4 +117,38 @@ EOF
git diff -b > out
test_expect_success 'another test, with -b' 'git diff expect out'
+test_expect_success 'check with no whitespace errors' '
+
+ git commit -m "snapshot" &&
+ echo "foo();" > x &&
+ git diff --check
+
+'
+
+test_expect_failure 'check with trailing whitespace' '
+
+ echo "foo(); " > x &&
+ git diff --check
+
+'
+
+test_expect_failure 'check with space before tab in indent' '
+
+ echo " foo();" > x &&
+ git diff --check
+
+'
+
+test_expect_failure '--check and --exit-code are exclusive' '
+
+ git diff --check --exit-code
+
+'
+
+test_expect_failure '--check and --quiet are exclusive' '
+
+ git diff --check --quiet
+
+'
+
test_done
--
1.5.3.7.1116.gf11de
^ permalink raw reply related
* [PATCH 1/2] Use "whitespace" consistently
From: Wincent Colaiuta @ 2007-12-12 8:12 UTC (permalink / raw)
To: git; +Cc: gitster, Wincent Colaiuta
In-Reply-To: <1197447122-93416-1-git-send-email-win@wincent.com>
For consistency, change "white space" and "whitespaces" to
"whitespace", fixing a couple of adjacent grammar problems in the
docs.
Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
Documentation/diff-options.txt | 12 ++++++------
Documentation/glossary.txt | 2 +-
diff.c | 4 ++--
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index d0154bb..5d22b7b 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -175,19 +175,19 @@ endif::git-format-patch[]
Shorthand for "--text".
--ignore-space-at-eol::
- Ignore changes in white spaces at EOL.
+ Ignore changes in whitespace at EOL.
--ignore-space-change::
- Ignore changes in amount of white space. This ignores white
- space at line end, and consider all other sequences of one or
- more white space characters to be equivalent.
+ Ignore changes in amount of whitespace. This ignores whitespace
+ at line end, and considers all other sequences of one or
+ more whitespace characters to be equivalent.
-b::
Shorthand for "--ignore-space-change".
--ignore-all-space::
- Ignore white space when comparing lines. This ignores
- difference even if one line has white space where the other
+ Ignore whitespace when comparing lines. This ignores
+ differences even if one line has whitespace where the other
line has none.
-w::
diff --git a/Documentation/glossary.txt b/Documentation/glossary.txt
index fc18744..f9f5a32 100644
--- a/Documentation/glossary.txt
+++ b/Documentation/glossary.txt
@@ -245,7 +245,7 @@ This commit is referred to as a "merge commit", or sometimes just a
of the object's contents using the Secure Hash Algorithm
1 and usually represented by the 40 character hexadecimal encoding of
the <<def_hash,hash>> of the object (possibly followed by
- a white space).
+ whitespace).
[[def_object_type]]object type::
One of the identifiers
diff --git a/diff.c b/diff.c
index f780e3e..e74a303 100644
--- a/diff.c
+++ b/diff.c
@@ -1015,7 +1015,7 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
if (line[i - 1] == '\t' && spaces)
space_before_tab = 1;
- /* check white space at line end */
+ /* check whitespace at line end */
if (line[len - 1] == '\n')
len--;
if (isspace(line[len - 1]))
@@ -1029,7 +1029,7 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
putchar(',');
}
if (white_space_at_end)
- printf("white space at end");
+ printf("whitespace at end");
printf(":%s ", reset);
emit_line_with_ws(1, set, reset, ws, line, len,
data->ws_rule);
--
1.5.3.7.1116.gf11de
^ permalink raw reply related
* [RFC PATCH 0/2] diff whitespace tweaks
From: Wincent Colaiuta @ 2007-12-12 8:12 UTC (permalink / raw)
To: git; +Cc: gitster
[1/2] Use "whitespace" consistently
This one is a simple consistency fix that should go on top of "master".
[2/2] "diff --check" should affect exit status
This one is why I put "RFC" in the subject line. Basically, when I
discovered that "diff --check" didn't affect the exit status of the
command I thought it might be a bug, in which case this fix or one like
it should go in for 1.5.4. This patch also applies on top of "master".
But if you don't consider that to be a bug then I guess it's "next"
or even "pu" material and I can bring it back up after 1.5.4 is out.
Another reason why I label this as "RFC" is that the diff machinery
is fairly bulky and I was not familiar with it. So this patch is the
result of someone who has looked at the source for half an hour and
deduced enough to implement the desired behaviour; but somone who
truly groks the diff code may be able to think of a nicer
implementation.
^ permalink raw reply
* [PATCH] Documentation: minor grammar fix for "git apply"
From: Wincent Colaiuta @ 2007-12-12 8:14 UTC (permalink / raw)
To: git; +Cc: gitster, Wincent Colaiuta
Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
Documentation/git-apply.txt | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index bae3e7b..9ec38f9 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -119,7 +119,7 @@ discouraged.
--no-add::
When applying a patch, ignore additions made by the
- patch. This can be used to extract common part between
+ patch. This can be used to extract the common part between
two files by first running `diff` on them and applying
the result with this option, which would apply the
deletion part but not addition part.
--
1.5.3.7.1116.gf11de
^ permalink raw reply related
* Re: git-cvsexportcommit fails for huge commits
From: Jeff King @ 2007-12-12 8:31 UTC (permalink / raw)
To: Markus Klinik; +Cc: git
In-Reply-To: <20071211200418.GA13815@mkl-desktop>
On Tue, Dec 11, 2007 at 09:04:18PM +0100, Markus Klinik wrote:
> git-cvsexportcommit fails for huge commits, that is commits with lots
> of files. To be exact, the problem arises if the generated 'cvs
> status' command exceeds the maximum length for commands.
>
> Here is the error:
>
> Can't exec "cvs": Argument list too long at
> /home/mkl/bin/git-cvsexportcommit line 334. Argument list too long 0
> at /home/mkl/bin/git-cvsexportcommit line 334. cvs status [snip:
> long, long list of files] 1792 at /home/mkl/bin/git-cvsexportcommit
> line 332.
Yuck. Unfortunately, CVS doesn't offer a more scalable interface, so we
are stuck splitting the arguments across multiple invocations.
However, I think this should work. The output of "cvs status foo bar" is
the same as "cvs status foo; cvs status bar". We will make our commits
in two CVS invocations, but since CVS isn't atomic _anyway_, we
shouldn't mind losing the atomicity.
Does the patch below clear up your problem?
---
diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl
index 92e4162..20e432b 100755
--- a/git-cvsexportcommit.perl
+++ b/git-cvsexportcommit.perl
@@ -195,11 +195,11 @@ foreach my $f (@files) {
my %cvsstat;
if (@canstatusfiles) {
if ($opt_u) {
- my @updated = safe_pipe_capture(@cvs, 'update', @canstatusfiles);
+ my @updated = xargs_safe_pipe_capture([@cvs, 'update'], @canstatusfiles);
print @updated;
}
my @cvsoutput;
- @cvsoutput= safe_pipe_capture(@cvs, 'status', @canstatusfiles);
+ @cvsoutput = xargs_safe_pipe_capture([@cvs, 'status'], @canstatusfiles);
my $matchcount = 0;
foreach my $l (@cvsoutput) {
chomp $l;
@@ -295,7 +295,7 @@ if ($dirtypatch) {
if ($opt_c) {
print "Autocommit\n $cmd\n";
- print safe_pipe_capture(@cvs, 'commit', '-F', '.msg', @files);
+ print xargs_safe_pipe_capture([@cvs, 'commit', '-F', '.msg'], @files);
if ($?) {
die "Exiting: The commit did not succeed";
}
@@ -335,6 +335,22 @@ sub safe_pipe_capture {
return wantarray ? @output : join('',@output);
}
+sub xargs_safe_pipe_capture {
+ my $MAX_ARG_LENGTH = 1024;
+ my $cmd = shift;
+ my @output;
+ while(@_) {
+ my @args;
+ my $length = 0;
+ while(@_ && $length < $MAX_ARG_LENGTH) {
+ push @args, shift;
+ $length += length($args[$#args]);
+ }
+ push @output, safe_pipe_capture(@$cmd, @args);
+ }
+ return @output;
+}
+
sub safe_pipe_capture_blob {
my $output;
if (my $pid = open my $child, '-|') {
^ permalink raw reply related
* Re: 'git fast-export' is crashing on the gcc repo
From: Junio C Hamano @ 2007-12-12 8:32 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Alex Riesen, git
In-Reply-To: <alpine.LFD.0.99999.0712112045040.555@xanadu.home>
Nicolas Pitre <nico@cam.org> writes:
> On Wed, 12 Dec 2007, Alex Riesen wrote:
>
>> Nicolas Pitre, Tue, Dec 11, 2007 23:06:42 +0100:
>> >
>> > Well, ignore the above. It seems that most of stdio doesn't set errno
>> > so the above is crap.
>> >
>>
>> Well, it had no reason to in this case. It's not an error.
>> It does not even have to do a syscall.
>
> Which is why I later agreed with your patch.
Still, I like your swapping of size and nmemb parameters, regardless of
the "don't bother calling fwrite(3) if size is zero" fix.
We are writing 1 element of size n-byte and expecting the call to return
1. It may be argued that writing one element of size 0-byte should
always write 1 element (without actually having to go down to write(2),
obviously) successfully and returning 0 from fwrite(3) is a bug ;-)
No, I am just kidding. I checked with POSIX and it clearly says it
should return 0 if size or nmemb is zero.
^ permalink raw reply
* Re: 'git fast-export' is crashing on the gcc repo
From: David Kastrup @ 2007-12-12 8:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nicolas Pitre, Alex Riesen, git
In-Reply-To: <7v3au89utj.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Nicolas Pitre <nico@cam.org> writes:
>
>> On Wed, 12 Dec 2007, Alex Riesen wrote:
>>
>>> Nicolas Pitre, Tue, Dec 11, 2007 23:06:42 +0100:
>>> >
>>> > Well, ignore the above. It seems that most of stdio doesn't set errno
>>> > so the above is crap.
>>> >
>>>
>>> Well, it had no reason to in this case. It's not an error.
>>> It does not even have to do a syscall.
>>
>> Which is why I later agreed with your patch.
>
> Still, I like your swapping of size and nmemb parameters, regardless
> of the "don't bother calling fwrite(3) if size is zero" fix.
I don't. Far too obscure, looks like an unintentional wart waiting to
be corrected.
I think that the explicit test is the way to go here.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: 'git fast-export' is crashing on the gcc repo
From: Junio C Hamano @ 2007-12-12 8:56 UTC (permalink / raw)
To: David Kastrup; +Cc: Nicolas Pitre, Alex Riesen, git
In-Reply-To: <854peo8fm5.fsf@lola.goethe.zz>
David Kastrup <dak@gnu.org> writes:
> Junio C Hamano <gitster@pobox.com> writes:
> ...
>> Still, I like your swapping of size and nmemb parameters, regardless
>> of the "don't bother calling fwrite(3) if size is zero" fix.
>
> I don't. Far too obscure, looks like an unintentional wart waiting to
> be corrected.
Oh, I did not mean it in the sense that would be a bugfix, but in the
sense that we are writing N instances of 1 byte, not 1 instance of N
byte blob, and should express size and nmemb parameters to fwrite(3) as
such. IOW, I would have preferred:
if (size && fwrite(buf, 1, size, stdout) != size)
barf(...);
^ permalink raw reply
* Re: [PATCH 1/2] git-help: add "help.format" config variable.
From: Junio C Hamano @ 2007-12-12 9:04 UTC (permalink / raw)
To: Christian Couder; +Cc: git
In-Reply-To: <20071212063320.3cbb1698.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> diff --git a/git.c b/git.c
> index 4f9876e..d46b63d 100644
> --- a/git.c
> +++ b/git.c
> @@ -324,7 +324,7 @@ static void handle_internal_command(int argc, const char **argv)
> { "gc", cmd_gc, RUN_SETUP },
> { "get-tar-commit-id", cmd_get_tar_commit_id },
> { "grep", cmd_grep, RUN_SETUP | USE_PAGER },
> - { "help", cmd_help },
> + { "help", cmd_help, RUN_SETUP },
> #ifndef NO_CURL
> { "http-fetch", cmd_http_fetch, RUN_SETUP },
> #endif
It would be _NICE_ if we read configuration when we are in a git
repository, but I am afraid this change is unnice -- the users used to
be able to say "git help" from anywhere didn't they? Now they will get
"Not a git repository". It needs to do an optional repository
discovery, not a mandatory one RUN_SETUP causes.
^ 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