Git development
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] git reflog expire
From: Shawn Pearce @ 2006-12-19 10:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhcvsry2c.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> Shawn Pearce <spearce@spearce.org> writes:
> 
> > Of course that's not what the code does, because if either the
> > old or the new object is no longer in the ODB you are pruning away
> > the log entry.  I cannot however come up with a better name than
> > --expire-lost.  :-(
> 
> How about --expire-unreachable?

That actually does make more sense.  Those commits are what we would
traditionally have called unreachable commits, or dangling commits.
Hmm.  Flip a coin.  I'm probably as bad at UI as you are.  :-)
 
> To have 'expire' action as a subcommand to 'git-reflog' is from
> implementor's point of view, and is a horrible organization from
> the UI standpoint.  To the end users, it may be easier to have a
> single 'git-gc' command that runs these commands with reasonable
> set of defaults:
> 
>   - rerere gc
>   - reflog expire --all
>   - (possibly) repack -a -d
>   - prune
> 
> If we go that route, it probably is not even necessary to
> advertise that 'expire' is a subcommand of reflog.  The users
> would not run it from the command line; it is an implementation
> detail of 'git-gc' command.

Yes!  I agree with you completely here.  I'd also include 'pack-refs
--prune' to your list above, at least for refs/tags.  Those suckers
can eat up a lot of disk, slow down viualizers, and well, just
don't change.  Pack 'em!

> > Needing a subcommand like 'git reflog show HEAD' is just a lot
> > of typing[*1*].
> 
> I am very interested in seeing how 'git reflog show HEAD' would
> show the reflog entries.  I've tried showing it just like log
> family shows (it is reasonably easy; you build the list of revs
> out of reflog entries and feed them to 'git-show' machinery),
> and while it works, it is unusable for the purpose of seeing
> which ones are the lost ones (amended commits and rebased branch
> remnants).

At this point I'm going to wind up getting less than 4 hours of
sleep tonight (I'll be lucky if I get 3).  So I'm just going to skip
trying to draft out what I might want this to look like right now.
I'll try to sketch out a dump by hand from a couple of my own reflogs
and post 'em later tonight.  Ugh.  Tonight.  Its Tuesday already.
My body still thinks its Monday.

> The best I came up with is still my "show-branch --reflog" so
> far.  You really need to show not just the commit title but how
> they topologically relate to the commits on the surviving
> branch, and I think having something graphical or semi-graphical
> is a must.

Probably right.  But I can't make heads or tails out of that
output for just one topic branch whose reflog consists of only
the following:

  $ cut -f2 .git/logs/refs/heads/sp/mapwin 
  checkout: Created from origin^0
  am: Create read_or_die utility routine.
  reset --hard 106d710b
  am: Create read_or_die utility routine.
  reset --hard 
  rebase: Create read_or_die utility routine.

`show-branch --reflog --topics sp/mapwin` prints much shorter output
and almost says something useful, but not really.

-- 

^ permalink raw reply

* Re: git-diff & cg-diff behavior difference
From: Vincent Legoll @ 2006-12-19 10:43 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: git
In-Reply-To: <4727185d0612190233w26188a56t50ad49d2de58b88b@mail.gmail.com>

On 12/19/06, Vincent Legoll <vincent.legoll@gmail.com> wrote:
> That's what I wanted to know: if it was a deliberate behavior, a bug on
> my side, or on git's...

Errr, I meant cogito's...

-- 

^ permalink raw reply

* Re: [PATCH] gitk: pass --no-left-right to rev-list
From: Johannes Schindelin @ 2006-12-19 10:47 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Paul Mackerras, git
In-Reply-To: <20061219100534.GA9206@coredump.intra.peff.net>

Hi,

On Tue, 19 Dec 2006, Jeff King wrote:

> I think there are three fixes:
>   1. this patch (avoid using left-right)
>   2. gitk can detect and parse the new format
>   3. revert automatic use of left-right
> The third makes the most sense to me.

IMHO 2. is the way to go.

Ciao,

^ permalink raw reply

* Re: cloning the kernel - why long time in "Resolving 313037 deltas"
From: Andy Whitcroft @ 2006-12-19 10:24 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, Theodore Tso, Nicolas Pitre,
	Randal L. Schwartz, git
In-Reply-To: <7v1wmwtfmk.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
>> Hi,
>>
>> in a very unscientific test, without your patch local cloning of the 
>> LilyPond repo takes 1m33s (user), and with your patch (pread() instead of 
>> mmap()) it takes 1m13s (user). The real times are somewhat bogus, but 
>> still in favour of pread(), but only by 8 seconds instead of 20.
>>
>> This is on Linux 2.4.32.
> 
> Interesting.  Anybody have numbers from 2.6?

On my debian etch system:

Linux version 2.6.17-2-686 (Debian 2.6.17-9) (waldi@debian.org) (gcc 
version 4.1.2 20060901 (prerelease) (Debian 4.1.1-13)) #1 SMP Wed Sep 13 
16:34:10 UTC 2006

I did one run first (not included) to get things nice and warm, then 
three runs of each.  Overall the same as reported elsewhere marginally 
better with pread().  I guess you could say a 20-30% improvement in 
system time which isn't to be sniffed at.

-apw

mmap():

real    1m5.187s
user    1m0.844s
sys     0m2.900s

real    1m6.748s
user    1m0.868s
sys     0m3.064s

real    1m5.604s
user    1m0.760s
sys     0m3.124s


pread():

real    1m4.676s
user    1m0.168s
sys     0m2.340s

real    1m3.563s
user    0m59.796s
sys     0m2.248s

real    1m4.066s
user    1m0.156s
sys     0m2.304s

^ permalink raw reply

* Re: [PATCH 2/2] git reflog expire
From: Junio C Hamano @ 2006-12-19 11:08 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git
In-Reply-To: <20061219104032.GB21324@spearce.org>

Shawn Pearce <spearce@spearce.org> writes:

>> The best I came up with is still my "show-branch --reflog" so
>> far.  You really need to show not just the commit title but how
>> they topologically relate to the commits on the surviving
>> branch, and I think having something graphical or semi-graphical
>> is a must.
>
> Probably right.  But I can't make heads or tails out of that
> output for just one topic branch...

Here is what my private 'pu' look like:

! [pu@{0}] Merge branch 'jc/fsck-reflog' into pu
 ! [pu@{1}] Merge branch 'jc/pickaxe' into pu
  ! [pu@{2}] Merge branches 'jn/web', 'jc/explain', 'jc/web' and
   ! [pu@{3}] Merge branch 'jc/leftright' into next
    ! [pu@{4}] Merge branch 'jc/fsck-reflog' into pu
     ! [pu@{5}] Merge branch 'jc/pickaxe' into pu
      ! [pu@{6}] Merge branches 'jc/fsck-reflog', 'jc/clone', 'j
       ! [pu@{7}] Merge branch 'jc/blame' into next
--------
-        [pu@{0}] Merge branch 'jc/fsck-reflog' into pu
+        [pu@{0}^2] git reflog expire
+        [pu@{0}^2^] Move in_merge_bases() to commit.c
+        [pu@{0}^2~2] reflog: fix warning message.
--       [pu@{1}] Merge branch 'jc/pickaxe' into pu
---      [pu@{2}] Merge branches 'jn/web', 'jc/explain', 'jc/web
----     [pu@{3}] Merge branch 'jc/leftright' into next
++++     [pu@{3}^2] Revert "Make left-right automatic."
      ...
++++     [pu@{3}~2^2~3^2^] Revert "fix testsuite: make sure they
    -    [pu@{4}] Merge branch 'jc/fsck-reflog' into pu
+   +    [pu@{0}^2~3] Teach git-repack to preserve objects refer
    --   [pu@{5}] Merge branch 'jc/pickaxe' into pu
    ---  [pu@{6}] Merge branches 'jc/fsck-reflog', 'jc/clone', '
+++ +++  [pu@{2}^5] para-walk: walk n trees, index and working t
      ...
+   +++  [pu@{0}^2~5] add for_each_reflog_ent() iterator
-------- [pu@{7}] Merge branch 'jc/blame' into next

What matters most is the leftmost column.  Most of the time when
you are looking at reflog, you are looking for what's rewound
and lost.  Stretches of lines with symbols (e.g. pu@{3}^2 to
pu@{3}~2^2~3^2^, and pu@{0}^2 to pu@{0}^2~2) are not so
interesting [*1*].

The entry whose column have whitespace there are the lost ones
(rewound or amended).  pu@{4} used to be the tip of 'pu', but
after I whipped 'next' into a reasonable shape, I did "git
branch -f pu next" to bundle the tips of topics that are not
ready for 'next' into it (and all the rest pu@{3}, pu@{2},
pu@{1} and pu@{0} are sequence of merges into 'pu').


[Footnote]

*1* We might want to add an output filter that lets the command
omit such lines with the same set of reachable tips when showing
the reflog entries.



^ permalink raw reply

* [PATCH] gitweb: Show '...' links in "summary" view only if there are more items
From: Jakub Narebski @ 2006-12-19 11:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Robert Fitzsimons
In-Reply-To: <200612190159.24173.jnareb@gmail.com>

Show "..." links in "summary" view to shortlog, heads (if there are
any), and tags (if there are any) only if there are more items to show
than shown already.

This means that "..." link is shown below shortened shortlog if there
are more than 16 commits, "..." link below shortened heads list if
there are more than 16 heads refs (16 branches), "..." link below
shortened tags list if there are more than 16 tags.

Added some comments.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---

Jakub Narebski wrote:
> Junio C Hamano wrote:
>> Jakub Narebski <jnareb@gmail.com> writes:
>> 
>>> Actually, that is needed to implement checking if we have more than
>>> the number of commits to show to add '...' at the end only if there
>>> are some commits which we don't show.
[...]
>> By the way, I wonder how that $extra is omitted when $revlist is
>> longer than $to; it should be a trivial fix but it seems to me
>> that it is always spitted out with the current code.
> 
> We should check if we want to omit $extra, either in caller or
> in callee, the *_body subroutine itself.

And now it is done.

Slightly tested: on my clone (copy) of git repository, which more than
16 commits, more than 16 heads (most temporary, and no longer worked on,
few tracking branches) and more than 16 heads show all "..." as it should.
Test of freshly created repository shown no "..." for commits (only one
commit), no "..." for heads (only one default head 'master'), and no
tags list (no tags at all).

By the way, I have _NOT_ applied Robert Fitzsimons patch, but they
(this patch and Robert patch) should be not in conflict if we remove
last chunk of Robert's patch (this changing --count=17 to --count=15
in git_summary).

 gitweb/gitweb.perl |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4059894..73877f2 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2915,8 +2915,9 @@ sub git_summary {
 	my $owner = git_get_project_owner($project);
 
 	my $refs = git_get_references();
-	my @taglist  = git_get_tags_list(15);
-	my @headlist = git_get_heads_list(15);
+	# we need to request one more than 16 (0..15) to check if those 16 are all
+	my @taglist  = git_get_tags_list(17);
+	my @headlist = git_get_heads_list(17);
 	my @forklist;
 	my ($check_forks) = gitweb_check_feature('forks');
 
@@ -2952,6 +2953,7 @@ sub git_summary {
 		}
 	}
 
+	# we need to request one more than 16 (0..15) to check if those 16 are all
 	open my $fd, "-|", git_cmd(), "rev-list", "--max-count=17",
 		git_get_head_hash($project), "--"
 		or die_error(undef, "Open git-rev-list failed");
@@ -2959,17 +2961,20 @@ sub git_summary {
 	close $fd;
 	git_print_header_div('shortlog');
 	git_shortlog_body(\@revlist, 0, 15, $refs,
+	                  $#revlist <=  15 ? undef :
 	                  $cgi->a({-href => href(action=>"shortlog")}, "..."));
 
 	if (@taglist) {
 		git_print_header_div('tags');
 		git_tags_body(\@taglist, 0, 15,
+		              $#taglist <=  15 ? undef :
 		              $cgi->a({-href => href(action=>"tags")}, "..."));
 	}
 
 	if (@headlist) {
 		git_print_header_div('heads');
 		git_heads_body(\@headlist, $head, 0, 15,
+		               $#headlist <= 15 ? undef :
 		               $cgi->a({-href => href(action=>"heads")}, "..."));
 	}
 
-- 

^ permalink raw reply related

* Re: branch rebase and push
From: Jakub Narebski @ 2006-12-19 11:58 UTC (permalink / raw)
  To: SungHyun Nam; +Cc: git
In-Reply-To: <4587BD31.9030804@gmail.com>

SungHyun Nam wrote:

> I'm sorry... but could you teach me how I publish the rebased branch?
> I think Junio does it for 'pu' branch.
> Is there recommended workflow? I cannot find.

Either have "Push:+refs/heads/br:refs/heads/br" in your
.git/remotes/<name> file, or it's equivalent config file
version, with '+' denoting that this branch sometimes does
not fast-forward.

Or use git push with --force option when you had rebased.

And of course the server you push to has to not have config variable 
receive.denyNonFastForwards set to true.
-- 
Jakub Narebski

^ permalink raw reply

* [PATCH] gitweb: Show '...' links in "summary" view only if there are more items
From: Robert Fitzsimons @ 2006-12-19 12:08 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Junio C Hamano, git, Robert Fitzsimons
In-Reply-To: <200612191214.58474.jnareb@gmail.com>

Show "..." links in "summary" view to shortlog, heads (if there are
any), and tags (if there are any) only if there are more items to show
than shown already.

This means that "..." link is shown below shortened shortlog if there
are more than 16 commits, "..." link below shortened heads list if
there are more than 16 heads refs (16 branches), "..." link below
shortened tags list if there are more than 16 tags.

Modified patch from Jakub to to apply cleanly to master, also preform
the same "..." link logic to the forks list.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Robert Fitzsimons <robfitz@273k.net>
---

> By the way, I have _NOT_ applied Robert Fitzsimons patch, but they
> (this patch and Robert patch) should be not in conflict if we remove
> last chunk of Robert's patch (this changing --count=17 to --count=15
> in git_summary).

Just removing the last chunk isn't correct, there are two slightly
different changes in that chuck.  The reduction in the max-count value
and a removal of a call to git_get_head_hash.

Here is an updated version of the patch which should apply against
master.

Robert



 gitweb/gitweb.perl |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3bee34c..8d409c7 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2918,8 +2918,9 @@ sub git_summary {
 	my $owner = git_get_project_owner($project);
 
 	my $refs = git_get_references();
-	my @taglist  = git_get_tags_list(15);
-	my @headlist = git_get_heads_list(15);
+	# we need to request one more than 16 (0..15) to check if those 16 are all
+	my @taglist  = git_get_tags_list(16);
+	my @headlist = git_get_heads_list(16);
 	my @forklist;
 	my ($check_forks) = gitweb_check_feature('forks');
 
@@ -2955,30 +2956,35 @@ sub git_summary {
 		}
 	}
 
-	open my $fd, "-|", git_cmd(), "rev-list", "--max-count=16",
+	# we need to request one more than 16 (0..15) to check if those 16 are all
+	open my $fd, "-|", git_cmd(), "rev-list", "--max-count=17",
 		$head, "--"
 		or die_error(undef, "Open git-rev-list failed");
 	my @revlist = map { chomp; $_ } <$fd>;
 	close $fd;
 	git_print_header_div('shortlog');
 	git_shortlog_body(\@revlist, 0, 15, $refs,
+	                  $#revlist <=  15 ? undef :
 	                  $cgi->a({-href => href(action=>"shortlog")}, "..."));
 
 	if (@taglist) {
 		git_print_header_div('tags');
 		git_tags_body(\@taglist, 0, 15,
+		              $#taglist <=  15 ? undef :
 		              $cgi->a({-href => href(action=>"tags")}, "..."));
 	}
 
 	if (@headlist) {
 		git_print_header_div('heads');
 		git_heads_body(\@headlist, $head, 0, 15,
+		               $#headlist <= 15 ? undef :
 		               $cgi->a({-href => href(action=>"heads")}, "..."));
 	}
 
 	if (@forklist) {
 		git_print_header_div('forks');
 		git_project_list_body(\@forklist, undef, 0, 15,
+		                      $#forklist <= 15 ? undef :
 		                      $cgi->a({-href => href(action=>"forks")}, "..."),
 				      'noheader');
 	}
-- 

^ permalink raw reply related

* Re: branch rebase and push
From: Johannes Schindelin @ 2006-12-19 12:13 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: SungHyun Nam, git
In-Reply-To: <200612191258.55226.jnareb@gmail.com>

Hi,

On Tue, 19 Dec 2006, Jakub Narebski wrote:

> SungHyun Nam wrote:
> 
> > I'm sorry... but could you teach me how I publish the rebased branch?
> > I think Junio does it for 'pu' branch.
> > Is there recommended workflow? I cannot find.
> 
> Either have "Push:+refs/heads/br:refs/heads/br" in your
> .git/remotes/<name> file, or it's equivalent config file
> version, with '+' denoting that this branch sometimes does
> not fast-forward.
> 
> Or use git push with --force option when you had rebased.
> 
> And of course the server you push to has to not have config variable 
> receive.denyNonFastForwards set to true.

If that is the case, there is a work-around, which happens to be also nice 
to tracking parties:

	$ git pull -s ours <remote> <branch>
	$ git push <remote> <branch>

The disadvantage (which I'd argue is actually an advantage) is that you do 
not lose history. After all: "those who do not learn from history are 
doomed to repeat it".

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] gitweb: Show '...' links in "summary" view only if there are more items
From: Jakub Narebski @ 2006-12-19 12:28 UTC (permalink / raw)
  To: Robert Fitzsimons; +Cc: Junio C Hamano, git
In-Reply-To: <20061219120854.GA16429@localhost>

Robert Fitzsimons wrote:
> Show "..." links in "summary" view to shortlog, heads (if there are
> any), and tags (if there are any) only if there are more items to show
> than shown already.
> 
> This means that "..." link is shown below shortened shortlog if there
> are more than 16 commits, "..." link below shortened heads list if
> there are more than 16 heads refs (16 branches), "..." link below
> shortened tags list if there are more than 16 tags.
> 
> Modified patch from Jakub to to apply cleanly to master, also preform
> the same "..." link logic to the forks list.

Junio usually puts such comments in brackets (I don't know if it is
always used, i.e. if it is some 'convention'), e.g.:

  Also perform the same "..." link logic to the forks list.

  [rf: Modified patch from Jakub to to apply cleanly to master]

or something like that. Just a nitpick.


By the way, it looks like git_get_projects_list($project) used to get
list of forks does not have any count limit option.

[...]
> ---
>
> > By the way, I have _NOT_ applied Robert Fitzsimons patch, but they
> > (this patch and Robert patch) should be not in conflict if we
> > remove last chunk of Robert's patch (this changing --count=17 to
> > --count=15 in git_summary).
>
> Just removing the last chunk isn't correct, there are two slightly
> different changes in that chuck.  The reduction in the max-count
> value and a removal of a call to git_get_head_hash.

The last chunk I meant to be removed was:

> @@ -2876,8 +2879,8 @@ sub git_summary {
>                 }
>         }
>  
> -       open my $fd, "-|", git_cmd(), "rev-list", "--max-count=17",
> -               git_get_head_hash($project), "--"
> +       open my $fd, "-|", git_cmd(), "rev-list", "--max-count=16",
> +               $head, "--"
>                 or die_error(undef, "Open git-rev-list failed");
>         my @revlist = map { chomp; $_ } <$fd>;
>         close $fd;

and if we remove that chunk, then your earlier patch would not
touch git_summary at all, so mine would cleanly apply (I think).

[...]
> -	my @taglist  = git_get_tags_list(15);
> -	my @headlist = git_get_heads_list(15);
> +	# we need to request one more than 16 (0..15) to check if those 16 are all
> +	my @taglist  = git_get_tags_list(16);
> +	my @headlist = git_get_heads_list(16);

It needs to be 17, not 16, otherwise we never would get "...". By default
we show _16_ items, from 0 to 15 inclusive, so we must get _17_ items
to check if there are more than 16.

>  	my @forklist;
>  	my ($check_forks) = gitweb_check_feature('forks');
>  
> @@ -2955,30 +2956,35 @@ sub git_summary {
>  		}
>  	}
>  
> -	open my $fd, "-|", git_cmd(), "rev-list", "--max-count=16",
> +	# we need to request one more than 16 (0..15) to check if those 16 are all
> +	open my $fd, "-|", git_cmd(), "rev-list", "--max-count=17",

Here you have 17.


>  	if (@forklist) {
>  		git_print_header_div('forks');
>  		git_project_list_body(\@forklist, undef, 0, 15,
> +		                      $#forklist <= 15 ? undef :
>  		                      $cgi->a({-href => href(action=>"forks")}, "..."),
>  				      'noheader');
>  	}

Nice catch. I forgot about this one.

-- 
Jakub Narebski

^ permalink raw reply

* Re: [PATCH] gitweb: Show '...' links in "summary" view only if there are more items
From: Jakub Narebski @ 2006-12-19 12:42 UTC (permalink / raw)
  To: Robert Fitzsimons; +Cc: Junio C Hamano, git
In-Reply-To: <200612191328.08928.jnareb@gmail.com>


>> @@ -2876,8 +2879,8 @@ sub git_summary {
>>                 }
>>         }
>>  
>> -       open my $fd, "-|", git_cmd(), "rev-list", "--max-count=17",
>> -               git_get_head_hash($project), "--"
>> +       open my $fd, "-|", git_cmd(), "rev-list", "--max-count=16",
>> +               $head, "--"
>>                 or die_error(undef, "Open git-rev-list failed");
>>         my @revlist = map { chomp; $_ } <$fd>;
>>         close $fd;
> 
> and if we remove that chunk, then your earlier patch would not
> touch git_summary at all, so mine would cleanly apply (I think).

Sorry, I haven't noticed git_get_head_hash($project) -> $head
Sorry for the noise.
-- 
Jakub Narebski

^ permalink raw reply

* Re: [PATCH] gitweb: Show '...' links in "summary" view only if there are more items
From: Robert Fitzsimons @ 2006-12-19 12:41 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Robert Fitzsimons, Junio C Hamano, git
In-Reply-To: <200612191328.08928.jnareb@gmail.com>

> Junio usually puts such comments in brackets (I don't know if it is
> always used, i.e. if it is some 'convention'), e.g.:
> 
>   Also perform the same "..." link logic to the forks list.
> 
>   [rf: Modified patch from Jakub to to apply cleanly to master]
> 
> or something like that. Just a nitpick.

No problem, I tried to find the approreati convention.

> > -	my @taglist  = git_get_tags_list(15);
> > -	my @headlist = git_get_heads_list(15);
> > +	# we need to request one more than 16 (0..15) to check if those 16 are all
> > +	my @taglist  = git_get_tags_list(16);
> > +	my @headlist = git_get_heads_list(16);
> 
> It needs to be 17, not 16, otherwise we never would get "...". By default
> we show _16_ items, from 0 to 15 inclusive, so we must get _17_ items
> to check if there are more than 16.

That was a copy error on my part.  Though looking at the code
git_get_tags_list and git_get_heads_list already adds one to the limit
value, so if you pass in 17 they will return 18 items.

Robert

^ permalink raw reply

* Re: [PATCH] Fix documentation copy&paste typo
From: Uwe Kleine-König @ 2006-12-19 14:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <11655782712452-git-send-email-zeisberg@informatik.uni-freiburg.de>

Hello Junio,

Uwe Kleine-König wrote:
> From: Uwe Zeisberger <zeisberg@informatik.uni-freiburg.de>
> 
> This was introduced in 45a3b12cfd3eaa05bbb0954790d5be5b8240a7b5
> 
> Signed-off-by: Uwe Kleine-König <zeisberg@informatik.uni-freiburg.de>
> ---
> [...]
you took that patch as bbee1d971dc07c29f840b439aa2a2c890a12cf9f, thanks
for that.

Somehow the 'ö' (o-umlaut) in my name was messed up.  If I do

	git cat-file -p bbee1d971dc07c29 | xxd | grep eine

I get:

	0000160: 6569 6e65 2d4b 1b2c 4143 361b 2842 6e69 eine-K.,AC6.(Bni

That is, the 'ö' became 8 byte long.  Can you tell me what went wrong
there?

The commits by Karl Hasselström <kha@treskal.com> (e.g. e67c66251a4165)
use UTF-8.

Does there exist a (maybe project specific) convention for the encoding
of commit logs?

Best regards
Uwe

-- 
Uwe Kleine-Koenig


^ permalink raw reply

* [PATCH] Enter interactive mode if no parameters are given to git-add
From: Andy Parkins @ 2006-12-19 15:16 UTC (permalink / raw)
  To: git

git-add with no parameters previously adds every modified file.  This
patch instead makes git-add enter interactive mode.

Signed-off-by: Andy Parkins <andyparkins@gmail.com>
---
This patch is against pu.

It seemed a bit nasty to add content without explicit instructions to do
so.  Instead, just drop into interactive mode, which they can easily get out
of if it was all a big accident, and additionally is quicker than typing
--interactive.

 builtin-add.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index aa2f0f3..52a63a0 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -96,10 +96,12 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		if (!strcmp("--interactive", argv[i]))
 			add_interactive++;
 	}
+	if (argc == 1)
+		add_interactive++;
 	if (add_interactive) {
 		const char *args[] = { "add--interactive", NULL };
 
-		if (add_interactive != 1 || argc != 2)
+		if (add_interactive != 1 || argc > 2)
 			die("add --interactive does not take any parameters");
 		execv_git_cmd(args);
 		exit(1);
-- 
1.4.4.2.g95ee-dirty

^ permalink raw reply related

* [PATCH] index-pack usage of mmap() is unacceptably slower on many OSes other than Linux
From: Nicolas Pitre @ 2006-12-19 15:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, Randal L. Schwartz, git
In-Reply-To: <7v1wmwtfmk.fsf@assigned-by-dhcp.cox.net>

It was reported by Randal L. Schwartz <merlyn@stonehenge.com> that 
indexing the Linux repository ~150MB pack takes about an hour on OS x 
while it's a minute on Linux.  It seems that the OS X mmap() 
implementation is more than 2 orders of magnitude slower than the Linux 
one.

Linus proposed a patch replacing mmap() with pread() bringing index-pack 
performance on OS X in line with the Linux one.  The performances on 
Linux also improved by a small margin.

Signed-off-by: Nicolas Pitre <nico@cam.org>

---

OK looks like this has been sorted out while I was away.  Good!

On Tue, 19 Dec 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Hi,
> >
> > in a very unscientific test, without your patch local cloning of the 
> > LilyPond repo takes 1m33s (user), and with your patch (pread() instead of 
> > mmap()) it takes 1m13s (user). The real times are somewhat bogus, but 
> > still in favour of pread(), but only by 8 seconds instead of 20.
> >
> > This is on Linux 2.4.32.
> 
> Interesting.  Anybody have numbers from 2.6?

My 37 seconds of yesterday dropped to 32.

This is Linus's patch plus a few cosmetic changes.

diff --git a/index-pack.c b/index-pack.c
index 6d6c92b..e08a687 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -1,3 +1,8 @@
+#define _XOPEN_SOURCE 500
+#include <unistd.h>
+#include <sys/time.h>
+#include <signal.h>
+
 #include "cache.h"
 #include "delta.h"
 #include "pack.h"
@@ -6,8 +11,6 @@
 #include "commit.h"
 #include "tag.h"
 #include "tree.h"
-#include <sys/time.h>
-#include <signal.h>
 
 static const char index_pack_usage[] =
 "git-index-pack [-v] [-o <index-file>] [{ ---keep | --keep=<msg> }] { <pack-file> | --stdin [--fix-thin] [<pack-file>] }";
@@ -87,7 +90,7 @@ static unsigned display_progress(unsigned n, unsigned total, unsigned last_pc)
 static unsigned char input_buffer[4096];
 static unsigned long input_offset, input_len, consumed_bytes;
 static SHA_CTX input_ctx;
-static int input_fd, output_fd, mmap_fd;
+static int input_fd, output_fd, pack_fd;
 
 /* Discard current buffer used content. */
 static void flush(void)
@@ -148,14 +151,14 @@ static const char *open_pack_file(const char *pack_name)
 			output_fd = open(pack_name, O_CREAT|O_EXCL|O_RDWR, 0600);
 		if (output_fd < 0)
 			die("unable to create %s: %s\n", pack_name, strerror(errno));
-		mmap_fd = output_fd;
+		pack_fd = output_fd;
 	} else {
 		input_fd = open(pack_name, O_RDONLY);
 		if (input_fd < 0)
 			die("cannot open packfile '%s': %s",
 			    pack_name, strerror(errno));
 		output_fd = -1;
-		mmap_fd = input_fd;
+		pack_fd = input_fd;
 	}
 	SHA1_Init(&input_ctx);
 	return pack_name;
@@ -279,27 +282,25 @@ static void *get_data_from_pack(struct object_entry *obj)
 {
 	unsigned long from = obj[0].offset + obj[0].hdr_size;
 	unsigned long len = obj[1].offset - from;
-	unsigned pg_offset = from % getpagesize();
-	unsigned char *map, *data;
+	unsigned char *src, *data;
 	z_stream stream;
 	int st;
 
-	map = mmap(NULL, len + pg_offset, PROT_READ, MAP_PRIVATE,
-		   mmap_fd, from - pg_offset);
-	if (map == MAP_FAILED)
-		die("cannot mmap pack file: %s", strerror(errno));
+	src = xmalloc(len);
+	if (pread(pack_fd, src, len, from) != len)
+		die("cannot pread pack file: %s", strerror(errno));
 	data = xmalloc(obj->size);
 	memset(&stream, 0, sizeof(stream));
 	stream.next_out = data;
 	stream.avail_out = obj->size;
-	stream.next_in = map + pg_offset;
+	stream.next_in = src;
 	stream.avail_in = len;
 	inflateInit(&stream);
 	while ((st = inflate(&stream, Z_FINISH)) == Z_OK);
 	inflateEnd(&stream);
 	if (st != Z_STREAM_END || stream.total_out != obj->size)
 		die("serious inflate inconsistency");
-	munmap(map, len + pg_offset);
+	free(src);
 	return data;
 }

^ permalink raw reply related

* Re: cloning the kernel - why long time in "Resolving 313037 deltas"
From: Theodore Tso @ 2006-12-19 16:19 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Nicolas Pitre, Linus Torvalds, Randal L. Schwartz, git
In-Reply-To: <20061219063930.GA2511@spearce.org>

On Tue, Dec 19, 2006 at 01:39:30AM -0500, Shawn Pearce wrote:
> This is why git-fast-import mmaps 128 MiB blocks from the file at
> a time.  The mmap region is usually much larger than the file itself;
> the application appends to the file via write() then goes back
> and rereads data when necessary via the already established mmap.
> Its rare for the application to need to unmap/remap a different block
> so there really isn't very much page table manipulation overhead.

Yes, but unless you are using the (non-portable, Linux specific)
MAP_POPULATE flag to mmap, each time you touch a new page, you end up
taking a page fault; and so malloc/read/free might *still* be faster.
I'd encourage you to make the change and benchmark it; the results may
be surprising.  I played with this with dcraw, the Canon Raw File
converter a while back (before MAP_POPULATE was added), where I found
that with a linear access pattern, if you are reading the entire file,
it's stil marginally faster to use read() over mmap(), because with
dcraw taking a page fault every 4k of raw file, the system time was
significantly higher.

So the main reason to use mamp, as Linus puts it, is if the management
overhead of needing to read lots of small bits of the file makes the
use of malloc/read to be a pain in the *ss, then go for it.  But don't
assume that you'll get better performance; in my experience, even on
the hyper-performant Linus kernel, mmap() in general only barely
breaks even with read().  On other systems, things are probably going
to be even worse.


^ permalink raw reply

* Re: [PATCH] Enter interactive mode if no parameters are given to git-add
From: Jakub Narebski @ 2006-12-19 16:42 UTC (permalink / raw)
  To: git
In-Reply-To: <200612191516.09606.andyparkins@gmail.com>

<opublikowany i wysłany>

Andy Parkins wrote:

> gmane.comp.version-control.git
> git-add with no parameters previously adds every modified file.  This
> patch instead makes git-add enter interactive mode.
> 
> Signed-off-by: Andy Parkins <andyparkins@gmail.com>
> ---
> This patch is against pu.
> 
> It seemed a bit nasty to add content without explicit instructions to do
> so.  Instead, just drop into interactive mode, which they can easily get out
> of if it was all a big accident, and additionally is quicker than typing
> --interactive.

Hmmm... --interactive is not documented, and neither is --all (erm, I think
git add -all oatch was not yet added).
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git


^ permalink raw reply

* Re: cloning the kernel - why long time in "Resolving 313037 deltas"
From: Linus Torvalds @ 2006-12-19 16:57 UTC (permalink / raw)
  To: Theodore Tso; +Cc: Shawn Pearce, Nicolas Pitre, Randal L. Schwartz, git
In-Reply-To: <20061219161919.GA16980@thunk.org>



On Tue, 19 Dec 2006, Theodore Tso wrote:
> 
> So the main reason to use mamp, as Linus puts it, is if the management
> overhead of needing to read lots of small bits of the file makes the
> use of malloc/read to be a pain in the *ss, then go for it.

An example of this in git is the regular pack-file accesses. We're MUCH 
better off just mmap'ing the whole pack-file (or at least big chunks of 
it) and not having to maintain difficult structures of "this is where I 
read that part of the file into memory", or read _big_ chunks when 
quite often we just use a few kB of it.

So mmap for pack-files does make sense, but probably only when you can 
mmap big chunks, and are going to access much smaller (random) parts of 
it.


^ permalink raw reply

* Re: [PATCH] Fix documentation copy&paste typo
From: Junio C Hamano @ 2006-12-19 17:27 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: git
In-Reply-To: <20061219141618.GA2539@cepheus>

Uwe Kleine-König <zeisberg@informatik.uni-freiburg.de> writes:

> I get:
>
> 	0000160: 6569 6e65 2d4b 1b2c 4143 361b 2842 6e69 eine-K.,AC6.(Bni
>
> That is, the 'ö' became 8 byte long.  Can you tell me what went wrong
> there?

Me, keyboard and Emacs screwed up and stored it in ISO-2022
instead of UTF-8.  Sorry.


^ permalink raw reply

* Re: [PATCH] Enter interactive mode if no parameters are given to git-add
From: Junio C Hamano @ 2006-12-19 18:00 UTC (permalink / raw)
  To: Andy Parkins; +Cc: git
In-Reply-To: <200612191516.09606.andyparkins@gmail.com>

Andy Parkins <andyparkins@gmail.com> writes:

> git-add with no parameters previously adds every modified file.  This
> patch instead makes git-add enter interactive mode.
> ...
> It seemed a bit nasty to add content without explicit instructions to do
> so.

Does anybody rely on the current behaviour?

It is nasty and I consider it a bug.  It appears that it has
been like that since the days when it was still a shell script.

While I think nobody should drive 'git add' from script, the
behaviour to add everything when there is no explicit parameter
(or fall back to interactive for that matter) has a problem if
you say "some command | xargs git-add --" (you need "xargs -r"
to fix it, which is GNU).

Let's first have a patch to exit with a warning message (but
status 0) without doing anything when there is no explicit
paths.

Again, does anybody rely on the current behaviour?


^ permalink raw reply

* Re: [PATCH] gitweb: Show '...' links in "summary" view only if there are more items
From: Junio C Hamano @ 2006-12-19 18:05 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Robert Fitzsimons, git
In-Reply-To: <200612191328.08928.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> Robert Fitzsimons wrote:
>> Show "..." links in "summary" view to shortlog, heads (if there are
>> any), and tags (if there are any) only if there are more items to show
>> than shown already.
>> 
>> This means that "..." link is shown below shortened shortlog if there
>> are more than 16 commits, "..." link below shortened heads list if
>> there are more than 16 heads refs (16 branches), "..." link below
>> shortened tags list if there are more than 16 tags.
>> 
>> Modified patch from Jakub to to apply cleanly to master, also preform
>> the same "..." link logic to the forks list.
>
> Junio usually puts such comments in brackets (I don't know if it is
> always used, i.e. if it is some 'convention'), e.g.:
>
>   Also perform the same "..." link logic to the forks list.
>
>   [rf: Modified patch from Jakub to to apply cleanly to master]

I would actually discourage this on messages that are still on
the list (i.e. not in commits).  I do [jc:] comment when I take
the proposed commit log message from the incoming e-mail
verbatim but I made modification to the patch text, because
otherwise the original submitter cannot tell from the log
message if that is meant to be identical to what was submitted.

I've only took a cursory look of the actual patch text, but what
Robert sent looked good to me.  Thanks, both.

^ permalink raw reply

* Re: StGit repo & gitweb, was Re: [PATCH] merge-recursive: add/add really  is modify/modify with an empty base
From: Petr Baudis @ 2006-12-19 18:50 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git, Catalin Marinas
In-Reply-To: <Pine.LNX.4.63.0612131232270.3635@wbgn013.biozentrum.uni-wuerzburg.de>

On Wed, Dec 13, 2006 at 12:46:56PM CET, Johannes Schindelin wrote:
> Not that it matters: repo.or.cz has a nice mirror. Pasky, how powerful is 
> that machine? I am a happy user of the gitweb interface on that box...

See http://repo.or.cz/about.html - two-processor P3 800MHz with 2G RAM,
and it does quite some work besides doing repo.or.cz as well - there are
no load problems so far, though. The gitweb load in particular is pretty
low, though - I think it's about 350 requests per day; the periodical
mirroring is far more intensive, but the intervals for this could be
reduced greatly if that would become a problem.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
The meaning of Stonehenge in Traflamadorian, when viewed from above, is:
"Replacement part being rushed with all possible speed."

^ permalink raw reply

* Re: [PATCH] index-pack usage of mmap() is unacceptably slower on many OSes other than Linux
From: Junio C Hamano @ 2006-12-19 19:00 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Linus Torvalds, Randal L. Schwartz, git
In-Reply-To: <Pine.LNX.4.64.0612191027270.18171@xanadu.home>

Nicolas Pitre <nico@cam.org> writes:

> OK looks like this has been sorted out while I was away.  Good!
>
> This is Linus's patch plus a few cosmetic changes.

Not a complaint but rather a request for free education ;-).

> diff --git a/index-pack.c b/index-pack.c
> index 6d6c92b..e08a687 100644
> --- a/index-pack.c
> +++ b/index-pack.c
> @@ -1,3 +1,8 @@
> +#define _XOPEN_SOURCE 500
> +#include <unistd.h>
> +#include <sys/time.h>
> +#include <signal.h>
> +
>  #include "cache.h"
>  #include "delta.h"
>  #include "pack.h"
> @@ -6,8 +11,6 @@
>  #include "commit.h"
>  #include "tag.h"
>  #include "tree.h"
> -#include <sys/time.h>
> -#include <signal.h>

Most of the rest of the sources seem to do our includes first
and source-file specific system includes at the end.  What's the
rationale for this change?

Do we need _XOPEN_SOURCE=500 because pread() is XSI?

Also nobody other than convert-objects.c has _XOPEN_SOURCE level
specified.  If _XOPEN_SOURCE matters I wonder if we should do so
in some central place to make it consistent across source files?

^ permalink raw reply

* Re: [PATCH] Add --remotes option to show-branch
From: Brian Gernhardt @ 2006-12-19 18:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vpsagtowm.fsf@assigned-by-dhcp.cox.net>


On Dec 19, 2006, at 12:50 AM, Junio C Hamano wrote:

> Actually, git show-branch origin/* would work equally well if
> you do not have local "heads/origin/*" branches.

Fair enough.  I'm not strongly attached to the options for the  
various refs.  And there is always showbranch.default.  I still think  
that --all would be good to keep around as meaning refs/heads/* and  
refs/remotes/*/*, however (and NOT refs/tags/*).  If nothing else, so  
a small-ish repository can compare everything fairly easily.


^ permalink raw reply

* Re: [PATCH] Enter interactive mode if no parameters are given to git-add
From: Linus Torvalds @ 2006-12-19 19:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andy Parkins, git
In-Reply-To: <7vy7p3pxyr.fsf@assigned-by-dhcp.cox.net>



On Tue, 19 Dec 2006, Junio C Hamano wrote:

> Andy Parkins <andyparkins@gmail.com> writes:
> 
> > git-add with no parameters previously adds every modified file.  This
> > patch instead makes git-add enter interactive mode.
> > ...
> > It seemed a bit nasty to add content without explicit instructions to do
> > so.
> 
> Does anybody rely on the current behaviour?

I don't know about "rely", but yes, it's what I normally do. When I import 
a tar-ball, I'll just do

	..untar..
	cd result-x.y.z
	git init-db
	git add
	git commit

and I don't usually use the "." at all, since it's unnecessary.

But hey, if it enters some interactive mode, maybe I can teach myself to 
either just do "git add .", or if the interactive mode has a choice of "a" 
for "all", then why not?

> While I think nobody should drive 'git add' from script, the
> behaviour to add everything when there is no explicit parameter
> (or fall back to interactive for that matter) has a problem if
> you say "some command | xargs git-add --" (you need "xargs -r"
> to fix it, which is GNU).

That's a really good argument against the current behaviour of "git add", 
so while I actually use it without any parameters, I will concur that we 
should probably change it.


^ 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