Git development
 help / color / mirror / Atom feed
* Re: Performance issue: initial git clone causes massive repack
From: Jakub Narebski @ 2009-04-07 13:37 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Björn Steinbrink, Sverre Rabbelier, david, Junio C Hamano,
	Nicolas Sebrecht, Robin H. Johnson, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0904070903020.6741@xanadu.home>

On Tue, 7 Apr 2009, Nicolas Pitre wrote:
> On Tue, 7 Apr 2009, Jakub Narebski wrote:
>> Björn Steinbrink <B.Steinbrink@gmx.de> writes:

[...]
>>> It's not primarily pack-objects, I think. It's the rev-list that's run
>>> by upload-pack.  Running "git rev-list --objects --all" on that repo
>>> eats about 2G RSS, easily killing the system's cache on a small box,
>>> leading to swapping and a painful time reading the packfile contents
>>> afterwards to send them to the client.
>> 
>> Than I think that "packfile caching" GSoC project (which is IIRC
>> "object enumeration caching", or at least includes it) should help
>> here.
> 
> NO!
> 
> Please people stop being so creative with all sort of ways to simply 
> avoid the real issue and focussing on a real fix.  Git has not become 
> what it is today by the accumulation of workarounds and ignorance of 
> fundamental issues.
> 
> Having git-rev-list consume about 2G RSS for the enumeration of 4M 
> objects is simply inacceptable, period.  This is the equivalent of 500 
> bytes per object pinned in memory on average, just for listing object, 
> which is completely silly. We ought to do better than that.

I have thought that the large amount of memory consumed by git-rev-list
was caused by not-so-sequential access to very large packfile (1.5GB+ if
I remember correctly), which I thought causes the whole packfile to be
mmapped and not only window, plus large amount of objects in 300MB+ mem
range or something; those both would account for around 2GB.

Besides even if git-rev-list wouldn't take so much memory, object
enumeration caching would still help with CPU load... admittedly less.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* [RFC PATCH] git-send-email.txt: clarify which options take an argument.
From: Matthieu Moy @ 2009-04-07 13:39 UTC (permalink / raw)
  To: gitster, git; +Cc: Matthieu Moy

---

Can someone more knowledgeable than me proof-read this? I tried to use
intuitive names for the arguments (like 'address' where an email
address is expected), but since there are options I never used, I may
have mis-understood some of them.

Thanks,

 Documentation/git-send-email.txt |   36 ++++++++++++++++++------------------
 1 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 10dfd66..e25da71 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -39,13 +39,13 @@ OPTIONS
 Composing
 ~~~~~~~~~
 
---bcc::
+--bcc='address'::
 	Specify a "Bcc:" value for each email. Default is the value of
 	'sendemail.bcc'.
 +
 The --bcc option must be repeated for each user you want on the bcc list.
 
---cc::
+--cc='address'::
 	Specify a starting "Cc:" value for each email.
 	Default is the value of 'sendemail.cc'.
 +
@@ -68,24 +68,24 @@ and In-Reply-To headers will be used unless they are removed.
 +
 Missing From or In-Reply-To headers will be prompted for.
 
---from::
+--from='address'::
 	Specify the sender of the emails.  This will default to
 	the value GIT_COMMITTER_IDENT, as returned by "git var -l".
 	The user will still be prompted to confirm this entry.
 
---in-reply-to::
+--in-reply-to='identifier'::
 	Specify the contents of the first In-Reply-To header.
 	Subsequent emails will refer to the previous email
 	instead of this if --chain-reply-to is set (the default)
 	Only necessary if --compose is also set.  If --compose
 	is not set, this will be prompted for.
 
---subject::
+--subject='string'::
 	Specify the initial subject of the email thread.
 	Only necessary if --compose is also set.  If --compose
 	is not set, this will be prompted for.
 
---to::
+--to='address'::
 	Specify the primary recipient of the emails generated. Generally, this
 	will be the upstream maintainer of the project involved. Default is the
 	value of the 'sendemail.to' configuration value; if that is unspecified,
@@ -97,7 +97,7 @@ The --to option must be repeated for each user you want on the to list.
 Sending
 ~~~~~~~
 
---envelope-sender::
+--envelope-sender='address'::
 	Specify the envelope sender used to send the emails.
 	This is useful if your default address is not the address that is
 	subscribed to a list. If you use the sendmail binary, you must have
@@ -105,12 +105,12 @@ Sending
 	the 'sendemail.envelopesender' configuration variable; if that is
 	unspecified, choosing the envelope sender is left to your MTA.
 
---smtp-encryption::
+--smtp-encryption='mode'::
 	Specify the encryption to use, either 'ssl' or 'tls'.  Any other
 	value reverts to plain SMTP.  Default is the value of
 	'sendemail.smtpencryption'.
 
---smtp-pass::
+--smtp-pass[='password']::
 	Password for SMTP-AUTH. The argument is optional: If no
 	argument is specified, then the empty string is used as
 	the password. Default is the value of 'sendemail.smtppass',
@@ -122,7 +122,7 @@ or on the command line. If a username has been specified (with
 specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
 user is prompted for a password while the input is masked for privacy.
 
---smtp-server::
+--smtp-server='host'::
 	If set, specifies the outgoing SMTP server to use (e.g.
 	`smtp.example.com` or a raw IP address).  Alternatively it can
 	specify a full pathname of a sendmail-like program instead;
@@ -132,7 +132,7 @@ user is prompted for a password while the input is masked for privacy.
 	`/usr/lib/sendmail` if such program is available, or
 	`localhost` otherwise.
 
---smtp-server-port::
+--smtp-server-port='port'::
 	Specifies a port different from the default port (SMTP
 	servers typically listen to smtp port 25 and ssmtp port
 	465). This can be set with 'sendemail.smtpserverport'.
@@ -140,7 +140,7 @@ user is prompted for a password while the input is masked for privacy.
 --smtp-ssl::
 	Legacy alias for '--smtp-encryption ssl'.
 
---smtp-user::
+--smtp-user='user'::
 	Username for SMTP-AUTH. Default is the value of 'sendemail.smtpuser';
 	if a username is not specified (with '--smtp-user' or 'sendemail.smtpuser'),
 	then authentication is not attempted.
@@ -149,13 +149,13 @@ user is prompted for a password while the input is masked for privacy.
 Automating
 ~~~~~~~~~~
 
---cc-cmd::
+--cc-cmd='cmd'::
 	Specify a command to execute once per patch file which
 	should generate patch file specific "Cc:" entries.
 	Output of this command must be single email address per line.
 	Default is the value of 'sendemail.cccmd' configuration value.
 
---[no-]chain-reply-to::
+--[no-]chain-reply-to='identifier'::
 	If this is set, each email will be sent as a reply to the previous
 	email sent.  If disabled with "--no-chain-reply-to", all emails after
 	the first will be sent as replies to the first email sent.  When using
@@ -163,7 +163,7 @@ Automating
 	entire patch series. Default is the value of the 'sendemail.chainreplyto'
 	configuration value; if that is unspecified, default to --chain-reply-to.
 
---identity::
+--identity='identity'::
 	A configuration identity. When given, causes values in the
 	'sendemail.<identity>' subsection to take precedence over
 	values in the 'sendemail' section. The default identity is
@@ -174,7 +174,7 @@ Automating
 	cc list. Default is the value of 'sendemail.signedoffbycc' configuration
 	value; if that is unspecified, default to --signed-off-by-cc.
 
---suppress-cc::
+--suppress-cc='category'::
 	Specify an additional category of recipients to suppress the
 	auto-cc of:
 +
@@ -211,7 +211,7 @@ specified, as well as 'body' if --no-signed-off-cc is specified.
 Administering
 ~~~~~~~~~~~~~
 
---confirm::
+--confirm='mode'::
 	Confirm just before sending:
 +
 --
@@ -234,7 +234,7 @@ have been specified, in which case default to 'compose'.
 	Make git-send-email less verbose.  One line per email should be
 	all that is output.
 
---[no-]validate::
+--[no-]validate='mode'::
 	Perform sanity checks on patches.
 	Currently, validation means the following:
 +
-- 
1.6.2.2.449.g92961.dirty

^ permalink raw reply related

* Re: Performance issue: initial git clone causes massive repack
From: Jon Smirl @ 2009-04-07 14:03 UTC (permalink / raw)
  To: Jakub Narebski
  Cc: Nicolas Pitre, Björn Steinbrink, Sverre Rabbelier, david,
	Junio C Hamano, Nicolas Sebrecht, Robin H. Johnson,
	Git Mailing List
In-Reply-To: <200904071537.04225.jnareb@gmail.com>

2009/4/7 Jakub Narebski <jnareb@gmail.com>:
> On Tue, 7 Apr 2009, Nicolas Pitre wrote:
>> On Tue, 7 Apr 2009, Jakub Narebski wrote:
>>> Björn Steinbrink <B.Steinbrink@gmx.de> writes:
>
> [...]
>>>> It's not primarily pack-objects, I think. It's the rev-list that's run
>>>> by upload-pack.  Running "git rev-list --objects --all" on that repo
>>>> eats about 2G RSS, easily killing the system's cache on a small box,
>>>> leading to swapping and a painful time reading the packfile contents
>>>> afterwards to send them to the client.
>>>
>>> Than I think that "packfile caching" GSoC project (which is IIRC
>>> "object enumeration caching", or at least includes it) should help
>>> here.
>>
>> NO!
>>
>> Please people stop being so creative with all sort of ways to simply
>> avoid the real issue and focussing on a real fix.  Git has not become
>> what it is today by the accumulation of workarounds and ignorance of
>> fundamental issues.
>>
>> Having git-rev-list consume about 2G RSS for the enumeration of 4M
>> objects is simply inacceptable, period.  This is the equivalent of 500
>> bytes per object pinned in memory on average, just for listing object,
>> which is completely silly. We ought to do better than that.
>
> I have thought that the large amount of memory consumed by git-rev-list
> was caused by not-so-sequential access to very large packfile (1.5GB+ if
> I remember correctly), which I thought causes the whole packfile to be
> mmapped and not only window, plus large amount of objects in 300MB+ mem
> range or something; those both would account for around 2GB.

I don't know all of the finer details of chasing revision lists, but
would it help if pack files recorded the root IDs of their object
trees at creation time and stored it in the front of the pack?


>
> Besides even if git-rev-list wouldn't take so much memory, object
> enumeration caching would still help with CPU load... admittedly less.
>
> --
> Jakub Narebski
> Poland
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* git rebase -i (and gitk) problem in Windows/Cygwin
From: Hannu Koivisto @ 2009-04-07 14:18 UTC (permalink / raw)
  To: git

Greetings,

With current git.git (1.6.2.2.446.gfbdc0) built for Cygwin, running
on Windows XP, executing the following commands...

mkdir test
cd test
git init

echo initial > kala.c
echo initial > sur.c
git add *.c
git commit -m "Initial commit."

echo addition >> kala.c
git commit -a -m "Kala addition 1."

echo addition >> sur.c
git commit -a -m "Sur addition."

echo addition2 >> kala.c
git commit -a -m "Kala addition 2."

git rebase -i HEAD~3

...and moving commit "Kala addition 2." right after "Kala addition
1." and marking it to be squashed results to

---8<----------------------------------------------------
error: Entry 'kala.c' not uptodate. Cannot merge.
fatal: merging of trees 787519579d90e45dfee00189985fa8c92f56be8f and 83f124d88764604c7d348e73103168bd98665e56 failed

Could not apply 14eb9c7... Kala addition 2.
---8<----------------------------------------------------

rebase -i used to work fine earlier, but unfortunately I don't
remember which version I used back then (1.6.something).

This problem doesn't occur on Linux with the same git version.

I don't know if it might be related (I suppose it could be because
of that "...not uptodate" message) but I also see the following
behaviour with gitk:

* I change a file in workspace.
* I "Update" in gitk - I see the change.
* I undo the change.
* I "Update" in gitk - I see an empty change.
* "Reload" doesn't help - I still se an empty change.
* I run "git status" on the command line and then select "Update"
  in gitk -> now the change disappears.

-- 
Hannu

^ permalink raw reply

* Re: Performance issue: initial git clone causes massive repack
From: Björn Steinbrink @ 2009-04-07 14:21 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Jakub Narebski, Sverre Rabbelier, david, Junio C Hamano,
	Nicolas Sebrecht, Robin H. Johnson, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0904070903020.6741@xanadu.home>

On 2009.04.07 09:13:45 -0400, Nicolas Pitre wrote:
> On Tue, 7 Apr 2009, Jakub Narebski wrote:
> 
> > Björn Steinbrink <B.Steinbrink@gmx.de> writes:
> > > On 2009.04.05 23:24:27 -0400, Nicolas Pitre wrote:
> > > > On Sun, 5 Apr 2009, Sverre Rabbelier wrote:
> > > > > 
> > > > > I agree here, we should either say "look, we don't really support big
> > > > > repositories because [explanation here], unless you [workarounds
> > > > > here]" OR we should work to improve the support we do have. Of course,
> > > > > the latter option does not magically create developer time to work on
> > > > > that, but if we do go that way we should at least tell people that we
> > > > > are aware of the problems and that it's on the global TODO list (not
> > > > > necessarily on anyone's personal TODO list though).
> > > > 
> > > > For the record... I at least am aware of the problem and it is indeed on 
> > > > my personal git todo list.  Not that I have a clear solution yet (I've 
> > > > been pondering on some git packing issues for almost 4 years now).
> > > > 
> > > > Still, in this particular case, the problem appears to be unclear to me, 
> > > > like "this shouldn't be so bad".
> > > 
> > > It's not primarily pack-objects, I think. It's the rev-list that's run
> > > by upload-pack.  Running "git rev-list --objects --all" on that repo
> > > eats about 2G RSS, easily killing the system's cache on a small box,
> > > leading to swapping and a painful time reading the packfile contents
> > > afterwards to send them to the client.
> > 
> > Than I think that "packfile caching" GSoC project (which is IIRC
> > "object enumeration caching", or at least includes it) should help
> > here.
> 
> NO!
> 
> Please people stop being so creative with all sort of ways to simply 
> avoid the real issue and focussing on a real fix.  Git has not become 
> what it is today by the accumulation of workarounds and ignorance of 
> fundamental issues.
> 
> Having git-rev-list consume about 2G RSS for the enumeration of 4M 
> objects is simply inacceptable, period.  This is the equivalent of 500 
> bytes per object pinned in memory on average, just for listing object, 
> which is completely silly. We ought to do better than that.

Ah, crap, I might have been fooled by "ps aux", top actually shows about
1.3G being shared, likely the mmapped pack files. And that will be
reused, assuming the box has enough memory to keep all that stuff.

But that's still 700MB or about 150 bytes per object on average.

A "struct tree" is 40 bytes here, adding the average path length (19 in
this repo) that's 59 byte, leaving about 90 bytes of "overhead" per
object, as end the end we seem to care only about the sha1 and the path
name.

And in the upload-pack case, there's also pack-objects running
concurrently, already going up to 950M RSS/100M shared _while_ the
rev-list is still running. So that's 3G of memory usage (2G if you
ignore the shared stuff) before the "Compressing objects" part even
starts. And of course, pack-objects will apparently start to mmap the
pack files only after the rev-list finished, so a "smart" OS might have
removed a lot of the mmapped stuff from memory again, causing it to be
re-read. :-/

Björn

^ permalink raw reply

* Re: git rebase -i (and gitk) problem in Windows/Cygwin
From: Johannes Schindelin @ 2009-04-07 14:25 UTC (permalink / raw)
  To: Hannu Koivisto; +Cc: git
In-Reply-To: <831vs4im37.fsf@kalahari.s2.org>

Hi,

On Tue, 7 Apr 2009, Hannu Koivisto wrote:

> * I change a file in workspace.
> * I "Update" in gitk - I see the change.
> * I undo the change.
> * I "Update" in gitk - I see an empty change.
> * "Reload" doesn't help - I still se an empty change.
> * I run "git status" on the command line and then select "Update"
>   in gitk -> now the change disappears.

What does "git diff" say?  It may be an autocrlf issue or a file mode 
issue.

Ciao,
Dscho

^ permalink raw reply

* Re: [RFC/PATCH 0/2] New 'stage' command
From: Octavio Alvarez @ 2009-04-07 15:01 UTC (permalink / raw)
  To: David Kågedal, Junio C. Hamano
  Cc: git, David Aguilar, Sverre Rabbelier, markus heidelberg,
	Felipe Contreras
In-Reply-To: <878wmcj1fs.fsf@krank.kagedal.org>

On Tue, 07 Apr 2009 01:46:47 -0700, David Kågedal <davidk@lysator.liu.se> wrote:

> "Octavio Alvarez" <alvarezp@alvarezp.ods.org> writes:
>
>> The difference between git diff and git reset is that git diff should  
>> take
>> a range of trees, not a range of commits as parameters. OTOH, git reset
>> doesn't know or care about trees, it needs commits.
>
> No, git diff doesn't take a range. It takes two trees (including the
> work "tree" and the index "tree"). The A..B syntax is nonsense for git
> diff, but I believe it supported for historical reasons.
>

Thanks.

I correct by s/range of// but the case persists.

^ permalink raw reply

* Re: [RFC PATCH] git-send-email.txt: clarify which options take an  argument.
From: Baz @ 2009-04-07 15:06 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: gitster, git
In-Reply-To: <1239111587-23452-1-git-send-email-Matthieu.Moy@imag.fr>

2009/4/7 Matthieu Moy <Matthieu.Moy@imag.fr>:
> ---
>
> Can someone more knowledgeable than me proof-read this? I tried to use
> intuitive names for the arguments (like 'address' where an email
> address is expected), but since there are options I never used, I may
> have mis-understood some of them.
>
> Thanks,
>
>  Documentation/git-send-email.txt |   36 ++++++++++++++++++------------------
>  1 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 10dfd66..e25da71 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -39,13 +39,13 @@ OPTIONS
>  Composing
>  ~~~~~~~~~
>
> ---bcc::
> +--bcc='address'::
>        Specify a "Bcc:" value for each email. Default is the value of
>        'sendemail.bcc'.

A general comment: the other man pages use a style like "--bcc=<address>".

>  +
>  The --bcc option must be repeated for each user you want on the bcc list.
>
> ---cc::
> +--cc='address'::
>        Specify a starting "Cc:" value for each email.
>        Default is the value of 'sendemail.cc'.
>  +
> @@ -68,24 +68,24 @@ and In-Reply-To headers will be used unless they are removed.
>  +
>  Missing From or In-Reply-To headers will be prompted for.
>
> ---from::
> +--from='address'::
>        Specify the sender of the emails.  This will default to
>        the value GIT_COMMITTER_IDENT, as returned by "git var -l".
>        The user will still be prompted to confirm this entry.
>
> ---in-reply-to::
> +--in-reply-to='identifier'::
>        Specify the contents of the first In-Reply-To header.
>        Subsequent emails will refer to the previous email
>        instead of this if --chain-reply-to is set (the default)
>        Only necessary if --compose is also set.  If --compose
>        is not set, this will be prompted for.
>
> ---subject::
> +--subject='string'::
>        Specify the initial subject of the email thread.
>        Only necessary if --compose is also set.  If --compose
>        is not set, this will be prompted for.
>
> ---to::
> +--to='address'::
>        Specify the primary recipient of the emails generated. Generally, this
>        will be the upstream maintainer of the project involved. Default is the
>        value of the 'sendemail.to' configuration value; if that is unspecified,
> @@ -97,7 +97,7 @@ The --to option must be repeated for each user you want on the to list.
>  Sending
>  ~~~~~~~
>
> ---envelope-sender::
> +--envelope-sender='address'::
>        Specify the envelope sender used to send the emails.
>        This is useful if your default address is not the address that is
>        subscribed to a list. If you use the sendmail binary, you must have
> @@ -105,12 +105,12 @@ Sending
>        the 'sendemail.envelopesender' configuration variable; if that is
>        unspecified, choosing the envelope sender is left to your MTA.
>
> ---smtp-encryption::
> +--smtp-encryption='mode'::
>        Specify the encryption to use, either 'ssl' or 'tls'.  Any other
>        value reverts to plain SMTP.  Default is the value of
>        'sendemail.smtpencryption'.

Maybe <encryption-mode> here. You use <mode> for other options below
to mean something else. However, I doubt the use is all that
confusing.

> ---smtp-pass::
> +--smtp-pass[='password']::
>        Password for SMTP-AUTH. The argument is optional: If no
>        argument is specified, then the empty string is used as
>        the password. Default is the value of 'sendemail.smtppass',
> @@ -122,7 +122,7 @@ or on the command line. If a username has been specified (with
>  specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
>  user is prompted for a password while the input is masked for privacy.
>
> ---smtp-server::
> +--smtp-server='host'::
>        If set, specifies the outgoing SMTP server to use (e.g.
>        `smtp.example.com` or a raw IP address).  Alternatively it can
>        specify a full pathname of a sendmail-like program instead;
> @@ -132,7 +132,7 @@ user is prompted for a password while the input is masked for privacy.
>        `/usr/lib/sendmail` if such program is available, or
>        `localhost` otherwise.
>
> ---smtp-server-port::
> +--smtp-server-port='port'::
>        Specifies a port different from the default port (SMTP
>        servers typically listen to smtp port 25 and ssmtp port
>        465). This can be set with 'sendemail.smtpserverport'.
> @@ -140,7 +140,7 @@ user is prompted for a password while the input is masked for privacy.
>  --smtp-ssl::
>        Legacy alias for '--smtp-encryption ssl'.
>
> ---smtp-user::
> +--smtp-user='user'::
>        Username for SMTP-AUTH. Default is the value of 'sendemail.smtpuser';
>        if a username is not specified (with '--smtp-user' or 'sendemail.smtpuser'),
>        then authentication is not attempted.
> @@ -149,13 +149,13 @@ user is prompted for a password while the input is masked for privacy.
>  Automating
>  ~~~~~~~~~~
>
> ---cc-cmd::
> +--cc-cmd='cmd'::
>        Specify a command to execute once per patch file which
>        should generate patch file specific "Cc:" entries.
>        Output of this command must be single email address per line.
>        Default is the value of 'sendemail.cccmd' configuration value.

Why not <command>? The option is short to save typing, doesn't mean
its documentation has to be.

> ---[no-]chain-reply-to::
> +--[no-]chain-reply-to='identifier'::
>        If this is set, each email will be sent as a reply to the previous
>        email sent.  If disabled with "--no-chain-reply-to", all emails after
>        the first will be sent as replies to the first email sent.  When using
> @@ -163,7 +163,7 @@ Automating
>        entire patch series. Default is the value of the 'sendemail.chainreplyto'
>        configuration value; if that is unspecified, default to --chain-reply-to.
>
> ---identity::
> +--identity='identity'::
>        A configuration identity. When given, causes values in the
>        'sendemail.<identity>' subsection to take precedence over
>        values in the 'sendemail' section. The default identity is
> @@ -174,7 +174,7 @@ Automating
>        cc list. Default is the value of 'sendemail.signedoffbycc' configuration
>        value; if that is unspecified, default to --signed-off-by-cc.
>
> ---suppress-cc::
> +--suppress-cc='category'::
>        Specify an additional category of recipients to suppress the
>        auto-cc of:
>  +
> @@ -211,7 +211,7 @@ specified, as well as 'body' if --no-signed-off-cc is specified.
>  Administering
>  ~~~~~~~~~~~~~
>
> ---confirm::
> +--confirm='mode'::
>        Confirm just before sending:
>  +
>  --
> @@ -234,7 +234,7 @@ have been specified, in which case default to 'compose'.
>        Make git-send-email less verbose.  One line per email should be
>        all that is output.
>
> ---[no-]validate::
> +--[no-]validate='mode'::
>        Perform sanity checks on patches.
>        Currently, validation means the following:

Is this one correct? Don't you just type --validate, or --no-validate,
without a value?

>  +
> --
> 1.6.2.2.449.g92961.dirty
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: git rebase -i (and gitk) problem in Windows/Cygwin
From: Hannu Koivisto @ 2009-04-07 14:53 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0904071624250.6897@intel-tinevez-2-302>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi,
>
> On Tue, 7 Apr 2009, Hannu Koivisto wrote:
>
>> * I change a file in workspace.
>> * I "Update" in gitk - I see the change.
>> * I undo the change.
>> * I "Update" in gitk - I see an empty change.
>> * "Reload" doesn't help - I still se an empty change.
>> * I run "git status" on the command line and then select "Update"
>>   in gitk -> now the change disappears.
>
> What does "git diff" say?  It may be an autocrlf issue or a file mode 
> issue.

It outputs only one line "diff --git a/path/to/file" (where the
file is the one I modified and then undid the modification).  After
I run "git status", it outputs absolutely nothing.

In my gitk problem case I have autocrlf and safecrlf toggled on for
the repository and crlf attribute forced for the file in question.
It was in this repository I also first saw the rebase problem but
when I ran the shown test case, autocrlf and safecrlf were off
(they are globally off and I just toggle them on for some
repositories).

I remember seeing and reporting a similar "spurious changes in
gitk" problem a long time ago and I recall back then Junio came up
with some file mode related modification.  I haven't seen the
problem since until now.

-- 
Hannu

^ permalink raw reply

* [[PATCH  1/1] Ensure consistent usage of mergetool.keepBackup
From: Ferry Huberts @ 2009-04-07 15:33 UTC (permalink / raw)
  To: git; +Cc: Ferry Huberts

In several places merge.keepBackup is used i.s.o.
mergetool.keepBackup. This patch makes it all
consistent.

Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
---
This patch is rebased on v1.6.2.2

 contrib/difftool/git-difftool.txt |    2 +-
 git-gui/git-gui.sh                |    2 +-
 git-gui/lib/mergetool.tcl         |    2 +-
 git-mergetool.sh                  |    2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/difftool/git-difftool.txt b/contrib/difftool/git-difftool.txt
index 6e2610c..ca7482a 100644
--- a/contrib/difftool/git-difftool.txt
+++ b/contrib/difftool/git-difftool.txt
@@ -66,7 +66,7 @@ merge.tool::
 +
 See the `--tool=<tool>` option above for more details.
 
-merge.keepBackup::
+mergetool.keepBackup::
 	The original, unedited file content can be saved to a file with
 	a `.orig` extension.  Defaults to `true` (i.e. keep the backup files).
 
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index e018e07..e4e643a 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -699,7 +699,7 @@ proc apply_config {} {
 
 set default_config(branch.autosetupmerge) true
 set default_config(merge.tool) {}
-set default_config(merge.keepbackup) true
+set default_config(mergetool.keepbackup) true
 set default_config(merge.diffstat) true
 set default_config(merge.summary) false
 set default_config(merge.verbosity) 2
diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl
index eb2b4b5..b7263b3 100644
--- a/git-gui/lib/mergetool.tcl
+++ b/git-gui/lib/mergetool.tcl
@@ -382,7 +382,7 @@ proc merge_tool_finish {fd} {
 		delete_temp_files $mtool_tmpfiles
 		ui_status [mc "Merge tool failed."]
 	} else {
-		if {[is_config_true merge.keepbackup]} {
+		if {[is_config_true mergetool.keepbackup]} {
 			file rename -force -- $backup "$mtool_target.orig"
 		}
 
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 87fa88a..1455bd9 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -430,7 +430,7 @@ else
 
     init_merge_tool_path "$merge_tool"
 
-    merge_keep_backup="$(git config --bool merge.keepBackup || echo true)"
+    merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)"
     merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
 
     if test -z "$merge_tool_cmd" && ! type "$merge_tool_path" > /dev/null 2>&1; then
-- 
1.6.0.6

^ permalink raw reply related

* Re: [RFC PATCH] git-send-email.txt: clarify which options take an  argument.
From: Baz @ 2009-04-07 15:48 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: gitster, git
In-Reply-To: <2faad3050904070806o55ba937cy1e5cd4fd156a4352@mail.gmail.com>

2009/4/7 Baz <brian.ewins@gmail.com>:
> 2009/4/7 Matthieu Moy <Matthieu.Moy@imag.fr>:
>> ---
>>
>> Can someone more knowledgeable than me proof-read this? I tried to use
>> intuitive names for the arguments (like 'address' where an email
>> address is expected), but since there are options I never used, I may
>> have mis-understood some of them.
>>
>> Thanks,
>>
>>  Documentation/git-send-email.txt |   36 ++++++++++++++++++------------------
>>  1 files changed, 18 insertions(+), 18 deletions(-)
>>
>> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
>> index 10dfd66..e25da71 100644
>> --- a/Documentation/git-send-email.txt
>> +++ b/Documentation/git-send-email.txt
>> @@ -39,13 +39,13 @@ OPTIONS
>>  Composing
>>  ~~~~~~~~~
>>
>> ---bcc::
>> +--bcc='address'::
>>        Specify a "Bcc:" value for each email. Default is the value of
>>        'sendemail.bcc'.
>
> A general comment: the other man pages use a style like "--bcc=<address>".
>
>>  +
>>  The --bcc option must be repeated for each user you want on the bcc list.
>>
>> ---cc::
>> +--cc='address'::
>>        Specify a starting "Cc:" value for each email.
>>        Default is the value of 'sendemail.cc'.
>>  +
>> @@ -68,24 +68,24 @@ and In-Reply-To headers will be used unless they are removed.
>>  +
>>  Missing From or In-Reply-To headers will be prompted for.
>>
>> ---from::
>> +--from='address'::
>>        Specify the sender of the emails.  This will default to
>>        the value GIT_COMMITTER_IDENT, as returned by "git var -l".
>>        The user will still be prompted to confirm this entry.
>>
>> ---in-reply-to::
>> +--in-reply-to='identifier'::
>>        Specify the contents of the first In-Reply-To header.
>>        Subsequent emails will refer to the previous email
>>        instead of this if --chain-reply-to is set (the default)
>>        Only necessary if --compose is also set.  If --compose
>>        is not set, this will be prompted for.
>>
>> ---subject::
>> +--subject='string'::
>>        Specify the initial subject of the email thread.
>>        Only necessary if --compose is also set.  If --compose
>>        is not set, this will be prompted for.
>>
>> ---to::
>> +--to='address'::
>>        Specify the primary recipient of the emails generated. Generally, this
>>        will be the upstream maintainer of the project involved. Default is the
>>        value of the 'sendemail.to' configuration value; if that is unspecified,
>> @@ -97,7 +97,7 @@ The --to option must be repeated for each user you want on the to list.
>>  Sending
>>  ~~~~~~~
>>
>> ---envelope-sender::
>> +--envelope-sender='address'::
>>        Specify the envelope sender used to send the emails.
>>        This is useful if your default address is not the address that is
>>        subscribed to a list. If you use the sendmail binary, you must have
>> @@ -105,12 +105,12 @@ Sending
>>        the 'sendemail.envelopesender' configuration variable; if that is
>>        unspecified, choosing the envelope sender is left to your MTA.
>>
>> ---smtp-encryption::
>> +--smtp-encryption='mode'::
>>        Specify the encryption to use, either 'ssl' or 'tls'.  Any other
>>        value reverts to plain SMTP.  Default is the value of
>>        'sendemail.smtpencryption'.
>
> Maybe <encryption-mode> here. You use <mode> for other options below
> to mean something else. However, I doubt the use is all that
> confusing.
>
>> ---smtp-pass::
>> +--smtp-pass[='password']::
>>        Password for SMTP-AUTH. The argument is optional: If no
>>        argument is specified, then the empty string is used as
>>        the password. Default is the value of 'sendemail.smtppass',
>> @@ -122,7 +122,7 @@ or on the command line. If a username has been specified (with
>>  specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
>>  user is prompted for a password while the input is masked for privacy.
>>
>> ---smtp-server::
>> +--smtp-server='host'::
>>        If set, specifies the outgoing SMTP server to use (e.g.
>>        `smtp.example.com` or a raw IP address).  Alternatively it can
>>        specify a full pathname of a sendmail-like program instead;
>> @@ -132,7 +132,7 @@ user is prompted for a password while the input is masked for privacy.
>>        `/usr/lib/sendmail` if such program is available, or
>>        `localhost` otherwise.
>>
>> ---smtp-server-port::
>> +--smtp-server-port='port'::
>>        Specifies a port different from the default port (SMTP
>>        servers typically listen to smtp port 25 and ssmtp port
>>        465). This can be set with 'sendemail.smtpserverport'.
>> @@ -140,7 +140,7 @@ user is prompted for a password while the input is masked for privacy.
>>  --smtp-ssl::
>>        Legacy alias for '--smtp-encryption ssl'.
>>
>> ---smtp-user::
>> +--smtp-user='user'::
>>        Username for SMTP-AUTH. Default is the value of 'sendemail.smtpuser';
>>        if a username is not specified (with '--smtp-user' or 'sendemail.smtpuser'),
>>        then authentication is not attempted.
>> @@ -149,13 +149,13 @@ user is prompted for a password while the input is masked for privacy.
>>  Automating
>>  ~~~~~~~~~~
>>
>> ---cc-cmd::
>> +--cc-cmd='cmd'::
>>        Specify a command to execute once per patch file which
>>        should generate patch file specific "Cc:" entries.
>>        Output of this command must be single email address per line.
>>        Default is the value of 'sendemail.cccmd' configuration value.
>
> Why not <command>? The option is short to save typing, doesn't mean
> its documentation has to be.
>
>> ---[no-]chain-reply-to::
>> +--[no-]chain-reply-to='identifier'::
>>        If this is set, each email will be sent as a reply to the previous
>>        email sent.  If disabled with "--no-chain-reply-to", all emails after
>>        the first will be sent as replies to the first email sent.  When using
>> @@ -163,7 +163,7 @@ Automating
>>        entire patch series. Default is the value of the 'sendemail.chainreplyto'
>>        configuration value; if that is unspecified, default to --chain-reply-to.
>>
>> ---identity::
>> +--identity='identity'::
>>        A configuration identity. When given, causes values in the
>>        'sendemail.<identity>' subsection to take precedence over
>>        values in the 'sendemail' section. The default identity is
>> @@ -174,7 +174,7 @@ Automating
>>        cc list. Default is the value of 'sendemail.signedoffbycc' configuration
>>        value; if that is unspecified, default to --signed-off-by-cc.
>>
>> ---suppress-cc::
>> +--suppress-cc='category'::
>>        Specify an additional category of recipients to suppress the
>>        auto-cc of:
>>  +
>> @@ -211,7 +211,7 @@ specified, as well as 'body' if --no-signed-off-cc is specified.
>>  Administering
>>  ~~~~~~~~~~~~~
>>
>> ---confirm::
>> +--confirm='mode'::
>>        Confirm just before sending:
>>  +
>>  --
>> @@ -234,7 +234,7 @@ have been specified, in which case default to 'compose'.
>>        Make git-send-email less verbose.  One line per email should be
>>        all that is output.
>>
>> ---[no-]validate::
>> +--[no-]validate='mode'::
>>        Perform sanity checks on patches.
>>        Currently, validation means the following:
>
> Is this one correct? Don't you just type --validate, or --no-validate,
> without a value?

Sorry, I should have stated that more clearly, since you said you're
unfamiliar with some of the options. This one is incorrect - you don't
pass an argument to --validate. The default value referred to is a
boolean, but its unclear from the text, I guess thats what misled you.
Following the examples in git-format-patch, the line below might be
better rewritten as:

If this is set, sanity checks will be performed on patches, overriding
the configuration setting of sendemail.validate. The configuration
setting defaults to true. Currently, validation means the following:
(etc)

>
>>  +
>> --
>> 1.6.2.2.449.g92961.dirty
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>

^ permalink raw reply

* Re: [RFC PATCH] git-send-email.txt: clarify which options take an  argument.
From: Matthieu Moy @ 2009-04-07 16:59 UTC (permalink / raw)
  To: Baz; +Cc: gitster, git
In-Reply-To: <2faad3050904070806o55ba937cy1e5cd4fd156a4352@mail.gmail.com>

Baz <brian.ewins@gmail.com> writes:

> 2009/4/7 Matthieu Moy <Matthieu.Moy@imag.fr>:
>> ---
>>
>> Can someone more knowledgeable than me proof-read this? I tried to use
>> intuitive names for the arguments (like 'address' where an email
>> address is expected), but since there are options I never used, I may
>> have mis-understood some of them.
>>
>> Thanks,
>>
>>  Documentation/git-send-email.txt |   36 ++++++++++++++++++------------------
>>  1 files changed, 18 insertions(+), 18 deletions(-)
>>
>> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
>> index 10dfd66..e25da71 100644
>> --- a/Documentation/git-send-email.txt
>> +++ b/Documentation/git-send-email.txt
>> @@ -39,13 +39,13 @@ OPTIONS
>>  Composing
>>  ~~~~~~~~~
>>
>> ---bcc::
>> +--bcc='address'::
>>        Specify a "Bcc:" value for each email. Default is the value of
>>        'sendemail.bcc'.
>
> A general comment: the other man pages use a style like "--bcc=<address>".

Almost all ;-). A quick grep showed me git-rev-list.txt and I did the
same. I'll send another patch to make it consistant.

>> +--smtp-encryption='mode'::
>>        Specify the encryption to use, either 'ssl' or 'tls'.  Any other
>>        value reverts to plain SMTP.  Default is the value of
>>        'sendemail.smtpencryption'.
>
> Maybe <encryption-mode> here. You use <mode> for other options below
> to mean something else. However, I doubt the use is all that
> confusing.

Changed to just "encryption", since this is the word used just below
in the sentence.

>> ---cc-cmd::
>> +--cc-cmd='cmd'::
>>        Specify a command to execute once per patch file which
>>        should generate patch file specific "Cc:" entries.
>>        Output of this command must be single email address per line.
>>        Default is the value of 'sendemail.cccmd' configuration value.
>
> Why not <command>? The option is short to save typing, doesn't mean
> its documentation has to be.

Done.

>> ---[no-]validate::
>> +--[no-]validate='mode'::
>>        Perform sanity checks on patches.
>>        Currently, validation means the following:
>
> Is this one correct? Don't you just type --validate, or --no-validate,
> without a value?

Fixed too.

Thanks, new patch follows.

-- 
Matthieu

^ permalink raw reply

* [PATCH 1/2] git-send-email.txt: clarify which options take an argument.
From: Matthieu Moy @ 2009-04-07 17:24 UTC (permalink / raw)
  To: gitster, git; +Cc: Matthieu Moy
In-Reply-To: <vpqvdpg8knl.fsf@bauges.imag.fr>

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/git-send-email.txt |   34 +++++++++++++++++-----------------
 1 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 10dfd66..0b1f183 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -39,13 +39,13 @@ OPTIONS
 Composing
 ~~~~~~~~~
 
---bcc::
+--bcc=<address>::
 	Specify a "Bcc:" value for each email. Default is the value of
 	'sendemail.bcc'.
 +
 The --bcc option must be repeated for each user you want on the bcc list.
 
---cc::
+--cc=<address>::
 	Specify a starting "Cc:" value for each email.
 	Default is the value of 'sendemail.cc'.
 +
@@ -68,24 +68,24 @@ and In-Reply-To headers will be used unless they are removed.
 +
 Missing From or In-Reply-To headers will be prompted for.
 
---from::
+--from=<address>::
 	Specify the sender of the emails.  This will default to
 	the value GIT_COMMITTER_IDENT, as returned by "git var -l".
 	The user will still be prompted to confirm this entry.
 
---in-reply-to::
+--in-reply-to=<identifier>::
 	Specify the contents of the first In-Reply-To header.
 	Subsequent emails will refer to the previous email
 	instead of this if --chain-reply-to is set (the default)
 	Only necessary if --compose is also set.  If --compose
 	is not set, this will be prompted for.
 
---subject::
+--subject=<string>::
 	Specify the initial subject of the email thread.
 	Only necessary if --compose is also set.  If --compose
 	is not set, this will be prompted for.
 
---to::
+--to=<address>::
 	Specify the primary recipient of the emails generated. Generally, this
 	will be the upstream maintainer of the project involved. Default is the
 	value of the 'sendemail.to' configuration value; if that is unspecified,
@@ -97,7 +97,7 @@ The --to option must be repeated for each user you want on the to list.
 Sending
 ~~~~~~~
 
---envelope-sender::
+--envelope-sender=<address>::
 	Specify the envelope sender used to send the emails.
 	This is useful if your default address is not the address that is
 	subscribed to a list. If you use the sendmail binary, you must have
@@ -105,12 +105,12 @@ Sending
 	the 'sendemail.envelopesender' configuration variable; if that is
 	unspecified, choosing the envelope sender is left to your MTA.
 
---smtp-encryption::
+--smtp-encryption=<encryption>::
 	Specify the encryption to use, either 'ssl' or 'tls'.  Any other
 	value reverts to plain SMTP.  Default is the value of
 	'sendemail.smtpencryption'.
 
---smtp-pass::
+--smtp-pass[=<password>]::
 	Password for SMTP-AUTH. The argument is optional: If no
 	argument is specified, then the empty string is used as
 	the password. Default is the value of 'sendemail.smtppass',
@@ -122,7 +122,7 @@ or on the command line. If a username has been specified (with
 specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
 user is prompted for a password while the input is masked for privacy.
 
---smtp-server::
+--smtp-server=<host>::
 	If set, specifies the outgoing SMTP server to use (e.g.
 	`smtp.example.com` or a raw IP address).  Alternatively it can
 	specify a full pathname of a sendmail-like program instead;
@@ -132,7 +132,7 @@ user is prompted for a password while the input is masked for privacy.
 	`/usr/lib/sendmail` if such program is available, or
 	`localhost` otherwise.
 
---smtp-server-port::
+--smtp-server-port=<port>::
 	Specifies a port different from the default port (SMTP
 	servers typically listen to smtp port 25 and ssmtp port
 	465). This can be set with 'sendemail.smtpserverport'.
@@ -140,7 +140,7 @@ user is prompted for a password while the input is masked for privacy.
 --smtp-ssl::
 	Legacy alias for '--smtp-encryption ssl'.
 
---smtp-user::
+--smtp-user=<user>::
 	Username for SMTP-AUTH. Default is the value of 'sendemail.smtpuser';
 	if a username is not specified (with '--smtp-user' or 'sendemail.smtpuser'),
 	then authentication is not attempted.
@@ -149,13 +149,13 @@ user is prompted for a password while the input is masked for privacy.
 Automating
 ~~~~~~~~~~
 
---cc-cmd::
+--cc-cmd=<command>::
 	Specify a command to execute once per patch file which
 	should generate patch file specific "Cc:" entries.
 	Output of this command must be single email address per line.
 	Default is the value of 'sendemail.cccmd' configuration value.
 
---[no-]chain-reply-to::
+--[no-]chain-reply-to=<identifier>::
 	If this is set, each email will be sent as a reply to the previous
 	email sent.  If disabled with "--no-chain-reply-to", all emails after
 	the first will be sent as replies to the first email sent.  When using
@@ -163,7 +163,7 @@ Automating
 	entire patch series. Default is the value of the 'sendemail.chainreplyto'
 	configuration value; if that is unspecified, default to --chain-reply-to.
 
---identity::
+--identity=<identity>::
 	A configuration identity. When given, causes values in the
 	'sendemail.<identity>' subsection to take precedence over
 	values in the 'sendemail' section. The default identity is
@@ -174,7 +174,7 @@ Automating
 	cc list. Default is the value of 'sendemail.signedoffbycc' configuration
 	value; if that is unspecified, default to --signed-off-by-cc.
 
---suppress-cc::
+--suppress-cc=<category>::
 	Specify an additional category of recipients to suppress the
 	auto-cc of:
 +
@@ -211,7 +211,7 @@ specified, as well as 'body' if --no-signed-off-cc is specified.
 Administering
 ~~~~~~~~~~~~~
 
---confirm::
+--confirm=<mode>::
 	Confirm just before sending:
 +
 --
-- 
1.6.2.2.449.g92961.dirty

^ permalink raw reply related

* [PATCH 2/2] git-rev-list.txt: make ascii markup uniform with other pages.
From: Matthieu Moy @ 2009-04-07 17:24 UTC (permalink / raw)
  To: gitster, git; +Cc: Matthieu Moy
In-Reply-To: <1239125073-4422-1-git-send-email-Matthieu.Moy@imag.fr>

Other pages use --option=<argument>, not --option='argument', do the
same here.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/rev-list-options.txt |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 7dd237c..11eec94 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -140,38 +140,38 @@ limiting may be applied.
 --
 
 -n 'number'::
---max-count='number'::
+--max-count=<number>::
 
 	Limit the number of commits output.
 
---skip='number'::
+--skip=<number>::
 
 	Skip 'number' commits before starting to show the commit output.
 
---since='date'::
---after='date'::
+--since=<date>::
+--after=<date>::
 
 	Show commits more recent than a specific date.
 
---until='date'::
---before='date'::
+--until=<date>::
+--before=<date>::
 
 	Show commits older than a specific date.
 
 ifdef::git-rev-list[]
---max-age='timestamp'::
---min-age='timestamp'::
+--max-age=<timestamp>::
+--min-age=<timestamp>::
 
 	Limit the commits output to specified time range.
 endif::git-rev-list[]
 
---author='pattern'::
---committer='pattern'::
+--author=<pattern>::
+--committer=<pattern>::
 
 	Limit the commits output to ones with author/committer
 	header lines that match the specified pattern (regular expression).
 
---grep='pattern'::
+--grep=<pattern>::
 
 	Limit the commits output to ones with log message that
 	matches the specified pattern (regular expression).
-- 
1.6.2.2.449.g92961.dirty

^ permalink raw reply related

* Re: Performance issue: initial git clone causes massive repack
From: Nicolas Pitre @ 2009-04-07 17:48 UTC (permalink / raw)
  To: Björn Steinbrink
  Cc: Jakub Narebski, Sverre Rabbelier, david, Junio C Hamano,
	Nicolas Sebrecht, Robin H. Johnson, Git Mailing List
In-Reply-To: <20090407142147.GA4413@atjola.homenet>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2250 bytes --]

On Tue, 7 Apr 2009, Björn Steinbrink wrote:

> On 2009.04.07 09:13:45 -0400, Nicolas Pitre wrote:
> > Having git-rev-list consume about 2G RSS for the enumeration of 4M 
> > objects is simply inacceptable, period.  This is the equivalent of 500 
> > bytes per object pinned in memory on average, just for listing object, 
> > which is completely silly. We ought to do better than that.
> 
> Ah, crap, I might have been fooled by "ps aux", top actually shows about
> 1.3G being shared, likely the mmapped pack files. And that will be
> reused, assuming the box has enough memory to keep all that stuff.

Right.  And since the pack is mapped read-only, it can be paged out 
easily by the OS.  And if that doesn't help, we already have 
core.packedGitWindowSize and core.packedGitLimit config options to play 
with.

> But that's still 700MB or about 150 bytes per object on average.
> 
> A "struct tree" is 40 bytes here, adding the average path length (19 in
> this repo) that's 59 byte, leaving about 90 bytes of "overhead" per
> object, as end the end we seem to care only about the sha1 and the path
> name.

I'm starting to think more seriously about pack v4 again, where each 
path components are indexed in a table.  Because most tree objects are 
different revisions of the same path, this could represent a significant 
saving in memory as well.

> And in the upload-pack case, there's also pack-objects running
> concurrently, already going up to 950M RSS/100M shared _while_ the
> rev-list is still running. So that's 3G of memory usage (2G if you
> ignore the shared stuff) before the "Compressing objects" part even
> starts. And of course, pack-objects will apparently start to mmap the
> pack files only after the rev-list finished, so a "smart" OS might have
> removed a lot of the mmapped stuff from memory again, causing it to be
> re-read. :-/

The first low hanging fruit to help this case is to make upload-pack use 
the --revs argument with pack-object to let it do the object enumeration 
itself directly, instead of relying on the rev-list output through a 
pipe.  This is what 'git repack' does already.  pack-objects has to 
access the pack anyway, so this would eliminate an extra access from a 
different process.


Nicolas

^ permalink raw reply

* Re: [PATCH v2 10/14] difftool: add various git-difftool tests
From: Markus Heidelberg @ 2009-04-07 17:52 UTC (permalink / raw)
  To: David Aguilar; +Cc: gitster, git, charles
In-Reply-To: <1239092483-14973-4-git-send-email-davvid@gmail.com>

David Aguilar, 07.04.2009:
> t7800-difftool.sh tests the various command-line flags,
> git-config variables, and environment settings supported by
> git-difftool.
> 
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
> 
> The original broke the test suite on Mac OS.
> Testing difftool.<path>.path is hard without internal support
> for a fake tool.

Mac OS doesn't have the "echo" command?

^ permalink raw reply

* Re: GPG signing for git commit?
From: Jakub Narebski @ 2009-04-07 17:55 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Chow Loong Jin, git
In-Reply-To: <alpine.LFD.2.00.0904031535140.3915@localhost.localdomain>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Sat, 4 Apr 2009, Chow Loong Jin wrote:
> > 
> > It crossed my mind that currently git commits cannot actually be
> > verified to be authentic, due to the fact that I can just set my
> > identity to be someone else, and then commit under their name.

[...]
> Btw, there's a final reason, and probably the really real one. Signing 
> each commit is totally stupid. It just means that you automate it, and you 
> make the signature worth less. It also doesn't add any real value, since 
> the way the git DAG-chain of SHA1's work, you only ever need _one_ 
> signature to make all the commits reachable from that one be effectively 
> covered by that one. So signing each commit is simply missing the point. 
> 
> IOW, you don't _ever_ have a reason to sign anythign but the "tip". The 
> only exception is the "go back and re-sign", but that's the one that 
> requires external signatures anyway.
> 
> So be happy with 'git tag -s'. It really is the right way.

And if you really, really need for some reason (for example
requirement checkpoint, or being paranoid enough) ned to have each and
every commit signed, you can use Monotone instead of Git.  That is
what we recommended IPsec (or something) on #git.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: Performance issue: initial git clone causes massive repack
From: Nicolas Pitre @ 2009-04-07 17:59 UTC (permalink / raw)
  To: Jakub Narebski
  Cc: Björn Steinbrink, Sverre Rabbelier, david, Junio C Hamano,
	Nicolas Sebrecht, Robin H. Johnson, Git Mailing List
In-Reply-To: <200904071537.04225.jnareb@gmail.com>

On Tue, 7 Apr 2009, Jakub Narebski wrote:

> On Tue, 7 Apr 2009, Nicolas Pitre wrote:
> > Having git-rev-list consume about 2G RSS for the enumeration of 4M 
> > objects is simply inacceptable, period.  This is the equivalent of 500 
> > bytes per object pinned in memory on average, just for listing object, 
> > which is completely silly. We ought to do better than that.
> 
> I have thought that the large amount of memory consumed by git-rev-list
> was caused by not-so-sequential access to very large packfile (1.5GB+ if
> I remember correctly), which I thought causes the whole packfile to be
> mmapped and not only window, plus large amount of objects in 300MB+ mem
> range or something; those both would account for around 2GB.

The pack has not to be mapped all at once.  At least on 32-bit machines 
the total pack mappings cannot exceed 256MB total by default.  On 64-bit 
machines the default is 8GB which might not work very well if total 
amount of RAM is lower than that.

Another consideration is the object layout in a pack.  Currently we have 
tree and blob objects mixed together so to have sequential pack access 
when performing a checkout.  Maybe having trees packed together would 
help a lot with object enumeration as the blobs have not to be mapped at 
all.  Remains to see how that might impact other operations though.

> Besides even if git-rev-list wouldn't take so much memory, object
> enumeration caching would still help with CPU load... admittedly less.

Yes, but let's not lose sight of all the inconvenients associated with 
extra caching.  If we can get away without it then all the better.


Nicolas

^ permalink raw reply

* Re: GPG signing for git commit?
From: Linus Torvalds @ 2009-04-07 18:04 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Chow Loong Jin, git
In-Reply-To: <m3ljqc1h55.fsf@localhost.localdomain>



On Tue, 7 Apr 2009, Jakub Narebski wrote:
> 
> And if you really, really need for some reason (for example
> requirement checkpoint, or being paranoid enough) ned to have each and
> every commit signed, you can use Monotone instead of Git.  That is
> what we recommended IPsec (or something) on #git.

Yeah, well..

That's more of a "If you really want to be incredibly slow, depend on an 
unbelievably baroque model, _and_ you are too stupid to understand the 
fact that you only need to sign the tip", then use Monotone.

But yes, the "sign each commit" is one of the big design mistakes in 
Monotone. Go ask them about how much pain it has caused them.

			Linus

^ permalink raw reply

* Re: [PATCH v3 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
From: Markus Heidelberg @ 2009-04-07 18:11 UTC (permalink / raw)
  To: David Aguilar; +Cc: gitster, git, charles
In-Reply-To: <1239092483-14973-6-git-send-email-davvid@gmail.com>

David Aguilar, 07.04.2009:
> This consolidates the common functionality from git-mergetool and
> git-difftool--helper into a single git-mergetool--lib scriptlet.
> 
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
> 
> Includes suggestions from Markus.
> We also moved opendiff up to the front so that it's
> preferred when on Mac OS.
> 
>  .gitignore                           |    1 +
>  Documentation/git-mergetool--lib.txt |   56 +++++
>  Makefile                             |    1 +
>  git-difftool--helper.sh              |  186 +----------------
>  git-mergetool--lib.sh                |  387 ++++++++++++++++++++++++++++++++++
>  git-mergetool.sh                     |  224 ++------------------
>  6 files changed, 467 insertions(+), 388 deletions(-)
>  create mode 100644 Documentation/git-mergetool--lib.txt
>  create mode 100644 git-mergetool--lib.sh
> 
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh

> +check_unchanged () {
> +	if merge_mode; then
> +		if test "$MERGED" -nt "$BACKUP"; then
> +			status=0
> +		else
> +			while true; do
> +				echo "$MERGED seems unchanged."
> +				printf "Was the merge successful? [y/n] "
> +				read answer < /dev/tty
> +				case "$answer" in
> +				y*|Y*) status=0; break ;;
> +				n*|N*) status=1; break ;;
> +				esac
> +			done
> +		fi
> +	else
> +		status=0
> +	fi
> +}

This function is now only called when $merge_mode is set.

> +run_merge_tool () {
> +	base_present="$2"
> +	if diff_mode; then
> +		status=0
> +		base_present="false"
> +	fi

$base_present also is only used when $merge_mode is set. And I think we
don't need to set status=0 conditionally then.

> +	case "$1" in
> +	kdiff3)
> +		if merge_mode; then
> +			if $base_present; then
> +				("$merge_tool_path" --auto \
> +					--L1 "$MERGED (Base)" \
> +					--L2 "$MERGED (Local)" \
> +					--L3 "$MERGED (Remote)" \
> +					-o "$MERGED" \
> +					"$BASE" "$LOCAL" "$REMOTE" \
> +				> /dev/null 2>&1)
> +			else
> +				("$merge_tool_path" --auto \
> +					--L1 "$MERGED (Local)" \
> +					--L2 "$MERGED (Remote)" \
> +					-o "$MERGED" \
> +					"$LOCAL" "$REMOTE" \
> +				> /dev/null 2>&1)
> +			fi
> +			status=$?
> +		else
> +			("$merge_tool_path" --auto \
> +			 --L1 "$MERGED (A)" \
> +			 --L2 "$MERGED (B)" "$LOCAL" "$REMOTE" \
> +			 > /dev/null 2>&1)
> +		fi
> +		;;
> +	kompare)
> +		"$merge_tool_path" "$LOCAL" "$REMOTE"
> +		;;
> +	tkdiff)
> +		if merge_mode; then
> +			if $base_present; then
> +				"$merge_tool_path" -a "$BASE" \
> +					-o "$MERGED" "$LOCAL" "$REMOTE"
> +			else
> +				"$merge_tool_path" \
> +					-o "$MERGED" "$LOCAL" "$REMOTE"
> +			fi
> +			status=$?
> +		else
> +			"$merge_tool_path" "$LOCAL" "$REMOTE"
> +		fi
> +		;;
> +	meld)
> +		if merge_mode; then
> +			touch "$BACKUP"
> +			"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
> +			check_unchanged
> +		else
> +			"$merge_tool_path" "$LOCAL" "$REMOTE"
> +		fi
> +		;;
> +	diffuse)
> +		if merge_mode; then
> +			touch "$BACKUP"
> +			if $base_present; then
> +				"$merge_tool_path" \
> +					"$LOCAL" "$MERGED" "$REMOTE" \
> +					"$BASE" | cat
> +			else
> +				"$merge_tool_path" \
> +					"$LOCAL" "$MERGED" "$REMOTE" | cat
> +			fi
> +			check_unchanged
> +		else
> +			"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
> +		fi
> +		;;
> +	vimdiff)
> +		if merge_mode; then
> +			touch "$BACKUP"
> +			"$merge_tool_path" -d -c "wincmd l" \
> +				"$LOCAL" "$MERGED" "$REMOTE"
> +			check_unchanged
> +		else
> +			"$merge_tool_path" -d -c "wincmd l" \
> +				"$LOCAL" "$REMOTE"
> +		fi
> +		;;
> +	gvimdiff)
> +		if merge_mode; then
> +			touch "$BACKUP"
> +			"$merge_tool_path" -d -c "wincmd l" -f \
> +				"$LOCAL" "$MERGED" "$REMOTE"
> +			check_unchanged
> +		else
> +			"$merge_tool_path" -d -c "wincmd l" -f \
> +				"$LOCAL" "$REMOTE"
> +		fi
> +		;;
> +	xxdiff)
> +		if merge_mode; then
> +			touch "$BACKUP"
> +			if $base_present; then
> +				"$merge_tool_path" -X --show-merged-pane \
> +					-R 'Accel.SaveAsMerged: "Ctrl-S"' \
> +					-R 'Accel.Search: "Ctrl+F"' \
> +					-R 'Accel.SearchForward: "Ctrl-G"' \
> +					--merged-file "$MERGED" \
> +					"$LOCAL" "$BASE" "$REMOTE"
> +			else
> +				"$merge_tool_path" -X $extra \
> +					-R 'Accel.SaveAsMerged: "Ctrl-S"' \
> +					-R 'Accel.Search: "Ctrl+F"' \
> +					-R 'Accel.SearchForward: "Ctrl-G"' \
> +					--merged-file "$MERGED" \
> +					"$LOCAL" "$REMOTE"
> +			fi
> +			check_unchanged
> +		else
> +			"$merge_tool_path" \
> +				-R 'Accel.Search: "Ctrl+F"' \
> +				-R 'Accel.SearchForward: "Ctrl-G"' \
> +				"$LOCAL" "$REMOTE"
> +		fi
> +		;;
> +	opendiff)
> +		if merge_mode; then
> +			touch "$BACKUP"
> +			if $base_present; then
> +				"$merge_tool_path" "$LOCAL" "$REMOTE" \
> +					-ancestor "$BASE" \
> +					-merge "$MERGED" | cat
> +			else
> +				"$merge_tool_path" "$LOCAL" "$REMOTE" \
> +					-merge "$MERGED" | cat
> +			fi
> +			check_unchanged
> +		else
> +			"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
> +		fi
> +		;;
> +	ecmerge)
> +		if merge_mode; then
> +			touch "$BACKUP"
> +			if $base_present; then
> +				"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \
> +					--default --mode=merge3 --to="$MERGED"
> +			else
> +				"$merge_tool_path" "$LOCAL" "$REMOTE" \
> +					--default --mode=merge2 --to="$MERGED"
> +			fi
> +			check_unchanged
> +		else
> +			"$merge_tool_path" "$LOCAL" "$REMOTE" \
> +				--default --mode=merge2 --to="$MERGED"
> +		fi
> +		;;
> +	emerge)
> +		if merge_mode; then
> +			if $base_present; then
> +				"$merge_tool_path" \
> +					-f emerge-files-with-ancestor-command \
> +					"$LOCAL" "$REMOTE" "$BASE" \
> +					"$(basename "$MERGED")"
> +			else
> +				"$merge_tool_path" \
> +					-f emerge-files-command \
> +					"$LOCAL" "$REMOTE" \
> +					"$(basename "$MERGED")"
> +			fi
> +			status=$?
> +		else
> +			"$merge_tool_path" -f emerge-files-command \
> +				"$LOCAL" "$REMOTE" "$(basename "$MERGED")"
> +		fi
> +		;;
> +	tortoisemerge)
> +		if $base_present; then
> +			touch "$BACKUP"
> +			"$merge_tool_path" \
> +				-base:"$BASE" -mine:"$LOCAL" \
> +				-theirs:"$REMOTE" -merged:"$MERGED"
> +			check_unchanged
> +		else
> +			echo "TortoiseMerge cannot be used without a base" 1>&2
> +			status=1
> +		fi
> +		;;

Looks nice.

> +	*)
> +		if test -z "$merge_tool_cmd"; then
> +			if merge_mode; then
> +				status=1
> +			fi
> +			break
> +		fi
> +		if merge_mode; then
> +			if test "$merge_tool_trust_exit_code" = "false"; then
> +				touch "$BACKUP"
> +				( eval $merge_tool_cmd )
> +				check_unchanged
> +			else
> +				( eval $merge_tool_cmd )
> +			fi
> +			status=$?

"fi" and "status=$?" have to be swapped, else it overwrites the value
set by check_unchanged():
+				status=$?
+			fi

> +		else
> +			( eval $merge_tool_cmd )
> +		fi
> +		;;
> +	esac
> +	return $status
> +}
> +
> +guess_merge_tool () {
> +	tools="ecmerge"
> +	if merge_mode; then
> +		tools="$tools tortoisemerge"
> +	else
> +		kompare=" kompare "
> +	fi
> +	if test -n "$DISPLAY"; then
> +		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
> +			tools="meld opendiff kdiff3""$kompare""tkdiff $tools"
> +			tools="$tools xxdiff gvimdiff diffuse"
> +		else
> +			tools="opendiff kdiff3""$kompare""tkdiff xxdiff $tools"
> +			tools="$tools meld gvimdiff diffuse"
> +		fi
> +	fi

The setting of $tools looks a bit weird. Maybe we can just ignore the
additional spaces here...

> +	if echo "${VISUAL:-$EDITOR}" | grep emacs > /dev/null 2>&1; then
> +		# $EDITOR is emacs so add emerge as a candidate
> +		tools="$tools emerge vimdiff"
> +	elif echo "${VISUAL:-$EDITOR}" | grep vim > /dev/null 2>&1; then
> +		# $EDITOR is vim so add vimdiff as a candidate
> +		tools="$tools vimdiff emerge"
> +	else
> +		tools="$tools emerge vimdiff"
> +	fi

...and delete them here with something like
    sed -e 's/ \+/ /g'
to have it look nice here?:

> +	echo >&2 "merge tool candidates: $tools"
> +
> +	# Loop over each candidate and stop when a valid merge tool is found.
> +	for i in $tools
> +	do
> +		merge_tool_path="$(translate_merge_tool_path "$i")"
> +		if type "$merge_tool_path" > /dev/null 2>&1; then
> +			merge_tool="$i"
> +			break
> +		fi
> +	done
> +
> +	if test -z "$merge_tool" ; then
> +		echo >&2 "No known merge resolution program available."
> +		return 1
> +	fi
> +	echo "$merge_tool"
> +}

^ permalink raw reply

* Re: Performance issue: initial git clone causes massive repack
From: Björn Steinbrink @ 2009-04-07 18:12 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Jakub Narebski, Sverre Rabbelier, david, Junio C Hamano,
	Nicolas Sebrecht, Robin H. Johnson, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0904071321520.6741@xanadu.home>

On 2009.04.07 13:48:02 -0400, Nicolas Pitre wrote:
> On Tue, 7 Apr 2009, Björn Steinbrink wrote:
> > And in the upload-pack case, there's also pack-objects running
> > concurrently, already going up to 950M RSS/100M shared _while_ the
> > rev-list is still running. So that's 3G of memory usage (2G if you
> > ignore the shared stuff) before the "Compressing objects" part even
> > starts. And of course, pack-objects will apparently start to mmap the
> > pack files only after the rev-list finished, so a "smart" OS might have
> > removed a lot of the mmapped stuff from memory again, causing it to be
> > re-read. :-/
> 
> The first low hanging fruit to help this case is to make upload-pack use 
> the --revs argument with pack-object to let it do the object enumeration 
> itself directly, instead of relying on the rev-list output through a 
> pipe.  This is what 'git repack' does already.  pack-objects has to 
> access the pack anyway, so this would eliminate an extra access from a 
> different process.

Hm, for an initial clone that would end up as:
git pack-objects --stdout --all
right?

If so, that doesn't look it it's going to work out as easily as one
would hope. Robin said that both processes, git-upload-pack (which does
the rev-list) and pack-objects peaked at ~2GB of RSS (which probably
includes the mmapped packs). But the above pack-objects with --all peaks
at 3.1G here, so it basically seems to keep all the stuff in memory that
the individual processes had. But this way, it's all at once, not 2G
first and then 2G in a second process, after the first one exitted.

Björn

^ permalink raw reply

* Re: git diff bug?
From: Antriksh Pany @ 2009-04-07 18:25 UTC (permalink / raw)
  To: David Abrahams; +Cc: Jeff King, git
In-Reply-To: <1BF9172A-BE91-4D1C-932A-EB9FD79BA402@boostpro.com>

I believe you need to have only the line

*.el diff=el

in the .gitattributes file.

The part

[diff = "el"]
   xfuncname = ...

has to be in the git config file. The .git/config file would do for this.


On Mon, Apr 6, 2009 at 2:39 PM, David Abrahams <dave@boostpro.com> wrote:
>
> On Apr 3, 2009, at 9:45 PM, Jeff King wrote:
>
>> On Fri, Apr 03, 2009 at 09:10:42PM -0400, David Abrahams wrote:
>>
>>> Please see
>>> http://github.com/techarcana/elisp/commit/63d672c296316c85690085930b05c642b88a9978#diff-2
>>>
>>> Note how the @@ ... @@ clauses are followed by text from the previous
>>> line's comment.  Not sure, but this strikes me as a line-ending issue.
>>> custom.el was originally built on a linux machine; now I'm using a Mac.
>>
>> This is as designed. The original file ("git show e7dd7db") contains (my
>> numbering seems different than what git produces; it is produced by "nl"
>> which is maybe treating some line endings differently earlier in the
>> file):
>>
>>  102   '(mm-attachment-override-types (quote ("text/x-vcard" "application/pkcs7-mime" "application/x-pkcs7-mime" "application/pkcs7-signature" "application/x-pkcs7-signature" "image/*")) nil nil "
>>  103  Added image/* to display attached images inline")
>>  104   '(mm-discouraged-alternatives (quote ("text/html" "text/richtext" "image/.*")) nil nil "
>>  105  The documentation for this variable says it all")
>>  106   '(mm-inline-text-html-with-images t)
>>  107   '(muse-project-alist (quote (("WikiPlanner" ("~/plans" :default "index" :major-mode planner-mode :visit-link planner-visit-link)))))
>>  108   '(org-agenda-files (quote ("~/organizer.org")))
>>
>> The changed text in your diff starts on 108. So we show 105-107 as
>> context lines. The text after the @@ clause is the "function header";
>> this is equivalent to "-p" in GNU diff. It's basically a guess about the
>> most interesting context to show, and looks alphabetic characters that
>> are left-aligned. In the case of lisp, it really isn't all that
>> interesting (and what looks so weird is that your file contains
>> a lot of
>>
>>  "\nSome text"
>>
>> so the text strings are all left-aligned. You can customize the regex
>> used to guess at the function header. See "defining a custom
>> hunk-header" in "git help attributes".
>
> Hmm, so I tried sticking this .gitattributes in my repo
>
>  *.el   diff=el
>  [diff "el"]
>        xfuncname = "^(\\(def[a-z]+ .+)$"
>
> and git diff barfed with
>
>  "el"] is not a valid attribute name: .gitattributes:2
>  "^(\\(def[a-z]+ is not a valid attribute name: .gitattributes:3
>
> What am I missing?  I tried googling, but from what turns up for me, it doesn't look like anyone else has ever tried to use this feature!
>
> TIA,
>
> --
> David Abrahams
> BoostPro Computing
> http://boostpro.com
>
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: Performance issue: initial git clone causes massive repack
From: Nicolas Pitre @ 2009-04-07 18:56 UTC (permalink / raw)
  To: Björn Steinbrink
  Cc: Jakub Narebski, Sverre Rabbelier, david, Junio C Hamano,
	Nicolas Sebrecht, Robin H. Johnson, Git Mailing List
In-Reply-To: <20090407181259.GB4413@atjola.homenet>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1201 bytes --]

On Tue, 7 Apr 2009, Björn Steinbrink wrote:

> On 2009.04.07 13:48:02 -0400, Nicolas Pitre wrote:
> > The first low hanging fruit to help this case is to make upload-pack use 
> > the --revs argument with pack-object to let it do the object enumeration 
> > itself directly, instead of relying on the rev-list output through a 
> > pipe.  This is what 'git repack' does already.  pack-objects has to 
> > access the pack anyway, so this would eliminate an extra access from a 
> > different process.
> 
> Hm, for an initial clone that would end up as:
> git pack-objects --stdout --all
> right?
> 
> If so, that doesn't look it it's going to work out as easily as one
> would hope. Robin said that both processes, git-upload-pack (which does
> the rev-list) and pack-objects peaked at ~2GB of RSS (which probably
> includes the mmapped packs). But the above pack-objects with --all peaks
> at 3.1G here, so it basically seems to keep all the stuff in memory that
> the individual processes had. But this way, it's all at once, not 2G
> first and then 2G in a second process, after the first one exitted.

Right, and it is probably faster too.

Can I get a copy of that repository somewhere?


Nicolas

^ permalink raw reply

* [PATCH] graph API: Added logic for colored edges
From: Allan Caffee @ 2009-04-07 18:57 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Jeff King, Nanako Shiraishi
In-Reply-To: <20090331235922.GA7411@linux.vnet>

Modified the graph drawing logic to colorize edges based on parent-child
relationships similiarly to gitk.

Signed-off-by: Allan Caffee <allan.caffee@gmail.com>
---
It seemed like it was time for another iteration so here it is.
Changes this round include: (Only the first two were actually posted
to the list as full patches.)

* graph.c: avoid compile warnings (Thanks Johannes)

* --graph: respect --no-color (Thanks Johannes)

* Remove some outdated TODO comments

* Avoid using forward declarations

* Remove commented out debugging code

* Avoid an unnecessary function call

* Refactor/rename some local functions
I replaced get_next_column_column color with graph_increment_column_color since
the return value is never actually used.  I also renamed
get_current_column_color to graph_get_current_column_color.

* Handle column colors as const pointers
The color codes used should never be modified once we begin graphing,
so columns should store/handle them as const char*.

* graph API: Add handling for merges with 3+ parents
I accidentally left this one out of the first round.

~Allan

 color.h |    1 +
 graph.c |  175 +++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 145 insertions(+), 31 deletions(-)

diff --git a/color.h b/color.h
index 6846be1..18abeb7 100644
--- a/color.h
+++ b/color.h
@@ -11,6 +11,7 @@
 #define GIT_COLOR_GREEN		"\033[32m"
 #define GIT_COLOR_YELLOW	"\033[33m"
 #define GIT_COLOR_BLUE		"\033[34m"
+#define GIT_COLOR_MAGENTA	"\033[35m"
 #define GIT_COLOR_CYAN		"\033[36m"
 #define GIT_COLOR_BG_RED	"\033[41m"
 
diff --git a/graph.c b/graph.c
index 162a516..0e112d7 100644
--- a/graph.c
+++ b/graph.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "commit.h"
+#include "color.h"
 #include "graph.h"
 #include "diff.h"
 #include "revision.h"
@@ -43,10 +44,6 @@ static void graph_show_strbuf(struct git_graph *graph, struct strbuf const *sb);
 
 /*
  * TODO:
- * - Add colors to the graph.
- *   Pick a color for each column, and print all characters
- *   in that column with the specified color.
- *
  * - Limit the number of columns, similar to the way gitk does.
  *   If we reach more than a specified number of columns, omit
  *   sections of some columns.
@@ -72,11 +69,21 @@ struct column {
 	 */
 	struct commit *commit;
 	/*
-	 * XXX: Once we add support for colors, struct column could also
-	 * contain the color of its branch line.
+	 * The color to (optionally) print this column in.
 	 */
+	const char *color;
 };
 
+static void strbuf_write_column(struct strbuf *sb, const struct column *c,
+		const char *s)
+{
+	if (c->color)
+		strbuf_addstr(sb, c->color);
+	strbuf_addstr(sb, s);
+	if (c->color)
+		strbuf_addstr(sb, GIT_COLOR_RESET);
+}
+
 enum graph_state {
 	GRAPH_PADDING,
 	GRAPH_SKIP,
@@ -86,6 +93,24 @@ enum graph_state {
 	GRAPH_COLLAPSING
 };
 
+/*
+ * The list of available column colors.
+ */
+static char column_colors[][COLOR_MAXLEN] = {
+	GIT_COLOR_RED,
+	GIT_COLOR_GREEN,
+	GIT_COLOR_YELLOW,
+	GIT_COLOR_BLUE,
+	GIT_COLOR_MAGENTA,
+	GIT_COLOR_CYAN,
+	GIT_COLOR_BOLD GIT_COLOR_RED,
+	GIT_COLOR_BOLD GIT_COLOR_GREEN,
+	GIT_COLOR_BOLD GIT_COLOR_YELLOW,
+	GIT_COLOR_BOLD GIT_COLOR_BLUE,
+	GIT_COLOR_BOLD GIT_COLOR_MAGENTA,
+	GIT_COLOR_BOLD GIT_COLOR_CYAN,
+};
+
 struct git_graph {
 	/*
 	 * The commit currently being processed
@@ -185,6 +210,11 @@ struct git_graph {
 	 * temporary array each time we have to output a collapsing line.
 	 */
 	int *new_mapping;
+	/*
+	 * The current default column color being used.  This is
+	 * stored as an index into the array column_colors.
+	 */
+	short default_column_color;
 };
 
 struct git_graph *graph_init(struct rev_info *opt)
@@ -201,6 +231,7 @@ struct git_graph *graph_init(struct rev_info *opt)
 	graph->num_columns = 0;
 	graph->num_new_columns = 0;
 	graph->mapping_size = 0;
+	graph->default_column_color = 0;
 
 	/*
 	 * Allocate a reasonably large default number of columns
@@ -312,6 +343,33 @@ static struct commit_list *first_interesting_parent(struct git_graph *graph)
 	return next_interesting_parent(graph, parents);
 }
 
+static const char* graph_get_current_column_color(const struct git_graph* graph)
+{
+	if (!DIFF_OPT_TST(&graph->revs->diffopt, COLOR_DIFF))
+		return NULL;
+	return column_colors[graph->default_column_color];
+}
+
+/*
+ * Update the graph's default column color.
+ */
+static void graph_increment_column_color(struct git_graph* graph)
+{
+	graph->default_column_color = (graph->default_column_color + 1) %
+		ARRAY_SIZE(column_colors);
+}
+
+static const char * graph_find_commit_color(const struct git_graph *graph,
+				      const struct commit *commit)
+{
+	int i;
+	for (i = 0; i < graph->num_columns; i++) {
+		if (graph->columns[i].commit == commit)
+			return graph->columns[i].color;
+	}
+	return graph_get_current_column_color(graph);
+}
+
 static void graph_insert_into_new_columns(struct git_graph *graph,
 					  struct commit *commit,
 					  int *mapping_index)
@@ -334,6 +392,7 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
 	 * This commit isn't already in new_columns.  Add it.
 	 */
 	graph->new_columns[graph->num_new_columns].commit = commit;
+	graph->new_columns[graph->num_new_columns].color = graph_find_commit_color(graph, commit);
 	graph->mapping[*mapping_index] = graph->num_new_columns;
 	*mapping_index += 2;
 	graph->num_new_columns++;
@@ -445,6 +504,12 @@ static void graph_update_columns(struct git_graph *graph)
 			for (parent = first_interesting_parent(graph);
 			     parent;
 			     parent = next_interesting_parent(graph, parent)) {
+				/*
+				 * If this is a merge increment the current
+				 * color.
+				 */
+				if (graph->num_parents > 1)
+					graph_increment_column_color(graph);
 				graph_insert_into_new_columns(graph,
 							      parent->item,
 							      &mapping_idx);
@@ -596,7 +661,7 @@ static void graph_output_padding_line(struct git_graph *graph,
 	 * Output a padding row, that leaves all branch lines unchanged
 	 */
 	for (i = 0; i < graph->num_new_columns; i++) {
-		strbuf_addstr(sb, "| ");
+		strbuf_write_column(sb, &graph->new_columns[i], "| ");
 	}
 
 	graph_pad_horizontally(graph, sb);
@@ -648,8 +713,11 @@ static void graph_output_pre_commit_line(struct git_graph *graph,
 	for (i = 0; i < graph->num_columns; i++) {
 		struct column *col = &graph->columns[i];
 		if (col->commit == graph->commit) {
+			struct strbuf tmp = STRBUF_INIT;
 			seen_this = 1;
-			strbuf_addf(sb, "| %*s", graph->expansion_row, "");
+			strbuf_addf(&tmp, "| %*s", graph->expansion_row, "");
+			strbuf_write_column(sb, col, tmp.buf);
+			strbuf_release(&tmp);
 		} else if (seen_this && (graph->expansion_row == 0)) {
 			/*
 			 * This is the first line of the pre-commit output.
@@ -662,13 +730,13 @@ static void graph_output_pre_commit_line(struct git_graph *graph,
 			 */
 			if (graph->prev_state == GRAPH_POST_MERGE &&
 			    graph->prev_commit_index < i)
-				strbuf_addstr(sb, "\\ ");
+				strbuf_write_column(sb, col, "\\ ");
 			else
-				strbuf_addstr(sb, "| ");
+				strbuf_write_column(sb, col, "| ");
 		} else if (seen_this && (graph->expansion_row > 0)) {
-			strbuf_addstr(sb, "\\ ");
+			strbuf_write_column(sb, col, "\\ ");
 		} else {
-			strbuf_addstr(sb, "| ");
+			strbuf_write_column(sb, col, "| ");
 		}
 	}
 
@@ -728,6 +796,7 @@ static void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)
 	 */
 	seen_this = 0;
 	for (i = 0; i <= graph->num_columns; i++) {
+		struct column *col = &graph->columns[i];
 		struct commit *col_commit;
 		if (i == graph->num_columns) {
 			if (seen_this)
@@ -744,14 +813,25 @@ static void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)
 			if (graph->num_parents < 3)
 				strbuf_addch(sb, ' ');
 			else {
+				/*
+				 * Here dashless_commits represents the
+				 * number of parents which don't need
+				 * to have dashes (because their edges
+				 * fit neatly under the commit).
+				 */
+				const int dashless_commits = 2;
 				int num_dashes =
-					((graph->num_parents - 2) * 2) - 1;
+					((graph->num_parents - dashless_commits) * 2) - 1;
 				for (j = 0; j < num_dashes; j++)
-					strbuf_addch(sb, '-');
-				strbuf_addstr(sb, ". ");
+					strbuf_write_column(sb,
+							    &graph->new_columns[(j / 2) + dashless_commits],
+							    "-");
+				strbuf_write_column(sb,
+						    &graph->new_columns[(j / 2) + dashless_commits],
+						    ". ");
 			}
 		} else if (seen_this && (graph->num_parents > 2)) {
-			strbuf_addstr(sb, "\\ ");
+			strbuf_write_column(sb, col, "\\ ");
 		} else if (seen_this && (graph->num_parents == 2)) {
 			/*
 			 * This is a 2-way merge commit.
@@ -768,11 +848,11 @@ static void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)
 			 */
 			if (graph->prev_state == GRAPH_POST_MERGE &&
 			    graph->prev_commit_index < i)
-				strbuf_addstr(sb, "\\ ");
+				strbuf_write_column(sb, col, "\\ ");
 			else
-				strbuf_addstr(sb, "| ");
+				strbuf_write_column(sb, col, "| ");
 		} else {
-			strbuf_addstr(sb, "| ");
+			strbuf_write_column(sb, col, "| ");
 		}
 	}
 
@@ -789,6 +869,17 @@ static void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)
 		graph_update_state(graph, GRAPH_COLLAPSING);
 }
 
+inline struct column* find_new_column_by_commit(struct git_graph *graph,
+						struct commit *commit)
+{
+	int i;
+	for (i = 0; i < graph->num_new_columns; i++) {
+		if (graph->new_columns[i].commit == commit)
+			return &graph->new_columns[i];
+	}
+	return 0;
+}
+
 static void graph_output_post_merge_line(struct git_graph *graph, struct strbuf *sb)
 {
 	int seen_this = 0;
@@ -798,24 +889,43 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct strbuf
 	 * Output the post-merge row
 	 */
 	for (i = 0; i <= graph->num_columns; i++) {
+		struct column *col = &graph->columns[i];
 		struct commit *col_commit;
 		if (i == graph->num_columns) {
 			if (seen_this)
 				break;
 			col_commit = graph->commit;
 		} else {
-			col_commit = graph->columns[i].commit;
+			col_commit = col->commit;
 		}
 
 		if (col_commit == graph->commit) {
+			/*
+			 * Since the current commit is a merge find
+			 * the columns for the parent commits in
+			 * new_columns and use those to format the
+			 * edges.
+			 */
+			struct commit_list *parents = NULL;
+			struct column *par_column;
 			seen_this = 1;
-			strbuf_addch(sb, '|');
-			for (j = 0; j < graph->num_parents - 1; j++)
-				strbuf_addstr(sb, "\\ ");
+			parents = first_interesting_parent(graph);
+			assert(parents);
+			par_column = find_new_column_by_commit(graph,parents->item);
+			assert(par_column);
+
+			strbuf_write_column(sb, par_column, "|");
+			for (j = 0; j < graph->num_parents - 1; j++) {
+				parents = next_interesting_parent(graph, parents);
+				assert(parents);
+				par_column = find_new_column_by_commit(graph,parents->item);
+				assert(par_column);
+				strbuf_write_column(sb, par_column, "\\ ");
+			}
 		} else if (seen_this) {
-			strbuf_addstr(sb, "\\ ");
+			strbuf_write_column(sb, col, "\\ ");
 		} else {
-			strbuf_addstr(sb, "| ");
+			strbuf_write_column(sb, col, "| ");
 		}
 	}
 
@@ -912,9 +1022,11 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct strbuf
 		if (target < 0)
 			strbuf_addch(sb, ' ');
 		else if (target * 2 == i)
-			strbuf_addch(sb, '|');
-		else
-			strbuf_addch(sb, '/');
+			strbuf_write_column(sb, &graph->new_columns[target], "|");
+		else {
+			strbuf_write_column(sb, &graph->new_columns[target], "/");
+
+		}
 	}
 
 	graph_pad_horizontally(graph, sb);
@@ -979,9 +1091,10 @@ static void graph_padding_line(struct git_graph *graph, struct strbuf *sb)
 	 * children that we have already processed.)
 	 */
 	for (i = 0; i < graph->num_columns; i++) {
-		struct commit *col_commit = graph->columns[i].commit;
+		struct column *col = &graph->columns[i];
+		struct commit *col_commit = col->commit;
 		if (col_commit == graph->commit) {
-			strbuf_addch(sb, '|');
+			strbuf_write_column(sb, col, "|");
 
 			if (graph->num_parents < 3)
 				strbuf_addch(sb, ' ');
@@ -991,7 +1104,7 @@ static void graph_padding_line(struct git_graph *graph, struct strbuf *sb)
 					strbuf_addch(sb, ' ');
 			}
 		} else {
-			strbuf_addstr(sb, "| ");
+			strbuf_write_column(sb, col, "| ");
 		}
 	}
 
-- 
1.5.6.3

^ permalink raw reply related

* Re: git over http not re-authenticating after 301 redirect?
From: Paul Vincent Craven @ 2009-04-07 19:25 UTC (permalink / raw)
  To: Martin Storsjö; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0904071041200.5901@localhost.localdomain>

On Tue, Apr 7, 2009 at 2:42 AM, Martin Storsjö <martin@martin.st> wrote:

> It's a long shot, but try see if you can upgrade to a newer version of
> libcurl; I think I saw some issue like this when I was experimenting with
> different http auth setups last week, that was solved by using newer
> versions...

No luck, but thanks for the suggestion. Updating to the March 3 build
of curl 7.19.4 yielded the same results.

-- 
Paul Vincent Craven
http://www.cravenfamily.com

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox