Git development
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] Teach receive-pack how to keep pack files based on object count.
From: Nicolas Pitre @ 2006-10-31 19:56 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20061031075704.GB7691@spearce.org>

On Tue, 31 Oct 2006, Shawn Pearce wrote:

> Since keeping a pushed pack or exploding it into loose objects
> should be a local repository decision this teaches receive-pack
> to decide if it should call unpack-objects or index-pack --stdin
> --fix-thin based on the setting of receive.unpackLimit and the
> number of objects contained in the received pack.

This works fine when used with my replacement patch for your [1/2] one.

> Currently this leaves every received pack as a kept pack.  We really
> don't want that as received packs will tend to be small.  Instead we
> want to delete the .keep file automatically after all refs have
> been updated.  That is being left as room for future improvement.

I think this should be solved before rx packs are actually stored as 
packs though.  Otherwise people will end up with unwanted .keep files 
left around.  Maybe having a much bigger default for object number 
treshold for the time being?  (unless this patch is applied to "next" at 
the same time as another one that actually deals with those .keep 
files).



^ permalink raw reply

* Re: [PATCH 1/2] Allow pack header preprocessing before unpack-objects/index-pack.
From: Nicolas Pitre @ 2006-10-31 19:33 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20061031075629.GA7691@spearce.org>

On Tue, 31 Oct 2006, Shawn Pearce wrote:

> However if the caller consumes the pack header from the input stream
> then its no longer available for unpack-objects or index-pack --stdin,
> both of which need the version and object count to process the stream.
> 
> This change introduces --pack_header=ver,cnt as a command line option
> that the caller can supply to indicate it has already consumed the
> pack header and what version and object count were found in that
> header.  As this option is only meant for low level applications
> such as receive-pack we are not documenting it at this time.

This breaks index-pack, and unpack-objects with OBJ_OFS_DELTA, if 
--pack-header is used.  The header is not accounted in the pack's offset 
and therefore every object's offset is wrong.

What about this patch instead?  This makes things much simpler IMHO.

diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index 74a90c1..e6d7574 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -371,6 +371,21 @@ int cmd_unpack_objects(int argc, const c
 				recover = 1;
 				continue;
 			}
+			if (!strncmp(arg, "--pack_header=", 14)) {
+				struct pack_header *hdr;
+				char *c;
+
+				hdr = (struct pack_header *)buffer;
+				hdr->hdr_signature = htonl(PACK_SIGNATURE);
+				hdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));
+				if (*c != ',')
+					die("bad %s", arg);
+				hdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));
+				if (*c)
+					die("bad %s", arg);
+				len = sizeof(*hdr);
+				continue;
+			}
 			usage(unpack_usage);
 		}
 
diff --git a/index-pack.c b/index-pack.c
index b37dd78..a3b55f9 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -841,6 +841,19 @@ int main(int argc, char **argv)
 				keep_msg = "";
 			} else if (!strncmp(arg, "--keep=", 7)) {
 				keep_msg = arg + 7;
+			} else if (!strncmp(arg, "--pack_header=", 14)) {
+				struct pack_header *hdr;
+				char *c;
+
+				hdr = (struct pack_header *)input_buffer;
+				hdr->hdr_signature = htonl(PACK_SIGNATURE);
+				hdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));
+				if (*c != ',')
+					die("bad %s", arg);
+				hdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));
+				if (*c)
+					die("bad %s", arg);
+				input_len = sizeof(*hdr);
 			} else if (!strcmp(arg, "-v")) {
 				verbose = 1;
 			} else if (!strcmp(arg, "-o")) {

^ permalink raw reply related

* Re: [PATCH 4/n] gitweb: Secure against commit-ish/tree-ish with the same name as path
From: Jakub Narebski @ 2006-10-31 16:53 UTC (permalink / raw)
  To: git
In-Reply-To: <200610302229.06957.jnareb@gmail.com>

Jakub Narebski wrote:
> @@ -2970,7 +2972,7 @@ sub git_tree {
>                 }
>         }
>         $/ = "\0";
> -       open my $fd, "-|", git_cmd(), "ls-tree", '-z', $hash
> +       open my $fd, "-|", git_cmd(), "ls-tree", '-z', $hash, "--"
>                 or die_error(undef, "Open git-ls-tree failed");
>         my @entries = map { chomp; $_ } <$fd>;
>         close $fd or die_error(undef, "Reading tree failed");

Please remove this chunk from patch!. It makes gitweb "tree" view
empty. I have forgot that git-ls-tree _requires_ <tree-ish> so there
is no way to mistake pathspec with <tree-ish>.

Bit overeager adding of "--"... 
-- 
Jakub Narebski

^ permalink raw reply

* [PATCH 8/n] gitweb: Fix two issues with quoted filenames in git_patchset_body
From: Jakub Narebski @ 2006-10-31 16:43 UTC (permalink / raw)
  To: git
In-Reply-To: <200610301953.01875.jnareb@gmail.com>

This fixes the following (minor) bugs:
* gitweb didn't remove quoted filenames from "git diff" header
* the filename in to-file +++ header used esc_html not esc_path

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index e0f7a3f..837b88e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2114,7 +2114,7 @@ sub git_patchset_body {
 			$patch_idx++;
 
 			# print "git diff" header
-			$patch_line =~ s!^(diff (.*?) )a/.*$!$1!;
+			$patch_line =~ s!^(diff (.*?) )"?a/.*$!$1!;
 			if ($from{'href'}) {
 				$patch_line .= $cgi->a({-href => $from{'href'}, -class => "path"},
 				                       'a/' . esc_path($from{'file'}));
@@ -2191,7 +2191,7 @@ sub git_patchset_body {
 				if ($to{'href'}) {
 					$patch_line = '+++ b/' .
 					              $cgi->a({-href=>$to{'href'}, -class=>"path"},
-					                      esc_html($to{'file'}));
+					                      esc_path($to{'file'}));
 				}
 				print "<div class=\"diff to_file\">$patch_line</div>\n";
 
-- 
1.4.3.3

^ permalink raw reply related

* [PATCH 7/n] gitweb: Output also empty patches in "commitdiff" view
From: Jakub Narebski @ 2006-10-31 16:36 UTC (permalink / raw)
  To: git
In-Reply-To: <200610301953.01875.jnareb@gmail.com>

Remove skipping over empty patches (i.e. patches which consist solely
of extended headers) in git_patchset_body, and add links to those
header-only patches in git_difftree_body (but not generate blobdiff
links when there were no change in file contents).

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

 gitweb/gitweb.perl |   67 +++++++++++++++++++++-------------------------------
 1 files changed, 27 insertions(+), 40 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 542dbca..e0f7a3f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1998,19 +1998,19 @@ sub git_difftree_body {
 			print "</td>\n";
 			print "<td>$mode_chnge</td>\n";
 			print "<td class=\"link\">";
-			if ($diff{'to_id'} ne $diff{'from_id'}) { # modified
-				if ($action eq 'commitdiff') {
-					# link to patch
-					$patchno++;
-					print $cgi->a({-href => "#patch$patchno"}, "patch");
-				} else {
-					print $cgi->a({-href => href(action=>"blobdiff",
-					                             hash=>$diff{'to_id'}, hash_parent=>$diff{'from_id'},
-					                             hash_base=>$hash, hash_parent_base=>$parent,
-					                             file_name=>$diff{'file'})},
-					              "diff");
-				}
-				print " | ";
+			if ($action eq 'commitdiff') {
+				# link to patch
+				$patchno++;
+				print $cgi->a({-href => "#patch$patchno"}, "patch") .
+				      " | ";
+			} elsif ($diff{'to_id'} ne $diff{'from_id'}) {
+				# "commit" view and modified file (not onlu mode changed)
+				print $cgi->a({-href => href(action=>"blobdiff",
+				                             hash=>$diff{'to_id'}, hash_parent=>$diff{'from_id'},
+				                             hash_base=>$hash, hash_parent_base=>$parent,
+				                             file_name=>$diff{'file'})},
+				              "diff") .
+				      " | ";
 			}
 			print $cgi->a({-href => href(action=>"blame", hash_base=>$hash,
 			                             file_name=>$diff{'file'})},
@@ -2038,19 +2038,19 @@ sub git_difftree_body {
 			              -class => "list"}, esc_path($diff{'from_file'})) .
 			      " with " . (int $diff{'similarity'}) . "% similarity$mode_chng]</span></td>\n" .
 			      "<td class=\"link\">";
-			if ($diff{'to_id'} ne $diff{'from_id'}) {
-				if ($action eq 'commitdiff') {
-					# link to patch
-					$patchno++;
-					print $cgi->a({-href => "#patch$patchno"}, "patch");
-				} else {
-					print $cgi->a({-href => href(action=>"blobdiff",
-					                             hash=>$diff{'to_id'}, hash_parent=>$diff{'from_id'},
-					                             hash_base=>$hash, hash_parent_base=>$parent,
-					                             file_name=>$diff{'to_file'}, file_parent=>$diff{'from_file'})},
-					              "diff");
-				}
-				print " | ";
+			if ($action eq 'commitdiff') {
+				# link to patch
+				$patchno++;
+				print $cgi->a({-href => "#patch$patchno"}, "patch") .
+				      " | ";
+			} elsif ($diff{'to_id'} ne $diff{'from_id'}) {
+				# "commit" view and modified file (not only pure rename or copy)
+				print $cgi->a({-href => href(action=>"blobdiff",
+				                             hash=>$diff{'to_id'}, hash_parent=>$diff{'from_id'},
+				                             hash_base=>$hash, hash_parent_base=>$parent,
+				                             file_name=>$diff{'to_file'}, file_parent=>$diff{'from_file'})},
+				              "diff") .
+				      " | ";
 			}
 			print $cgi->a({-href => href(action=>"blame", hash_base=>$parent,
 			                             file_name=>$diff{'from_file'})},
@@ -2072,7 +2072,6 @@ sub git_patchset_body {
 	my $patch_idx = 0;
 	my $in_header = 0;
 	my $patch_found = 0;
-	my $skip_patch = 0;
 	my $diffinfo;
 	my (%from, %to);
 
@@ -2084,8 +2083,6 @@ sub git_patchset_body {
 
 		if ($patch_line =~ m/^diff /) { # "git diff" header
 			# beginning of patch (in patchset)
-			$skip_patch = 0;
-
 			if ($patch_found) {
 				# close previous patch
 				print "</div>\n"; # class="patch"
@@ -2116,13 +2113,6 @@ sub git_patchset_body {
 			}
 			$patch_idx++;
 
-			# for now we skip empty patches
-			if ($diffinfo->{'from_id'} eq $diffinfo->{'to_id'}) { # no change
-				$in_header = 1;
-				$skip_patch = 0;
-				next LINE;
-			}
-
 			# print "git diff" header
 			$patch_line =~ s!^(diff (.*?) )a/.*$!$1!;
 			if ($from{'href'}) {
@@ -2143,10 +2133,7 @@ sub git_patchset_body {
 			print "<div class=\"diff extended_header\">\n";
 			$in_header = 1;
 			next LINE;
-		} else {
-			# actual skipping of empty patches
-			next LINE if $skip_patch;
-		} # start of patch in patchset
+		}
 
 		if ($in_header) {
 			if ($patch_line !~ m/^---/) {
-- 
1.4.3.3

^ permalink raw reply related

* Re: git fetch with multiple remotes failing?
From: Linus Torvalds @ 2006-10-31 16:32 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Git Mailing List
In-Reply-To: <20061031155913.GA5157@mellanox.co.il>



On Tue, 31 Oct 2006, Michael S. Tsirkin wrote:
> 
> $ cat .git/remotes/origin
> URL: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
> Pull: +refs/heads/master:refs/heads/linus_master
> $ cat .git/remotes/jejb-scsi-misc-2.6
> URL: git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi-misc-2.6.git
> Pull: +refs/heads/master:refs/heads/jejb-scsi-misc-2.6
> $ git fetch -f origin jejb-scsi-misc-2.6
> error: no such remote ref refs/heads/jejb-scsi-misc-2.6
> Fetch failure:
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
> 
> Looks I must give remotes one by one?

Yes. A single "fetch" will only ever connect to a single repository. If 
you want to fetch from multiple repositories, you have to do multiple 
connections, and thus multiple "fetch"es.

The protocol doesn't even really allow for connecting to two different 
repositories with one connection.

There's also a purely syntactic issue: when you say

	git fetch -f origin jejb-scsi-misc-2.6

then the "origin" is considered to specify the repository (and any 
"default branches") and any subsequent arguments are considered to 
override the _branch_ information in that repository. So the above command 
literally means "fetch branch 'jejb-scsi-misc-2.6' from the repository 
described by 'origin'".

Btw, this _syntactic_ issue is separate from the issue of actually 
initiating multiple connections. For example, "git push" actually 
understands that you can push multiple different repositories at once, but 
even then you can't specify it on the command line directly, for the exact 
same reason as the above "git fetch" thing: the first argument is 
considered the "repository" specifier, and any subsequent arguments are 
specifiers for which branches/tags to push.

So for "git push" (where it makes sense to push the same branches multiple 
times), you can actually do what I do:

 - .git/config contains:

	[remote "all"]
		url = master.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux-2.6
		url = login.osdl.org:linux-2.6.git

 - and not "git push all master" will push the "master" branch to _both_ 
   of those remote repositories.

However, even that doesn't really make sense for "git fetch": while it is 
a sensible operation to push the same branch-specifier to more than one 
repository, it does _not_ make sense to _fetch_ the same branch-specifier 
from more than one repository.

So multiple fetches would only make sense if you don't allow any branch 
specifiers (and then they'd be fetched from whatever branches the 
"remotes" file says). But that would make "git fetch" have two totally 
different modes, so that's not very good either.

And in the end, even a "git push all" that pushes to multiple repositories 
will actually end up connecting once for each repository, so it's really 
just a shorthand for doing multiple "git push"es. There's no real 
technical advantage, just a convenience.

So I'd suggest just doing

	git fetch origin
	git fetch jejb-scsi-misc-2.6

separately. Sadly, there's not even any way to fake this out with a git 
alias.


^ permalink raw reply

* [PATCH 6/n] gitweb: Remove redundant "blob" links from git_difftree_body
From: Jakub Narebski @ 2006-10-31 16:07 UTC (permalink / raw)
  To: git
In-Reply-To: <200610301953.01875.jnareb@gmail.com>

This revert parts of commit 4777b0141a4812177390da4b6ebc9d40ac3da4b5
  "gitweb: Restore object-named links in item lists"
by Petr Baudis, which bring back "blob" links in difftree-raw like
part of "commit" and "commitdiff" views, namely in git_difftree_body
subroutine. First, it did that incompletely: it did not add "blob"
link for added files, and added block used mixture of tabs and spaces
for align. Second, in "difftree" view the "blob" link is not the most
interesting, *contrary* to "blob"/"tree" link in "tree" view, so it
should be enough to have hidden link in the form of file name entry.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
I'm still for having those object-named links in "tree" view.

But perhaps it would be better to add "blob" links for all possible
statuses, including added files... Pasky, Luben, Junio?

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c22fcc5..542dbca 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1967,9 +1967,6 @@ sub git_difftree_body {
 				print $cgi->a({-href => "#patch$patchno"}, "patch");
 				print " | ";
 			}
-			print $cgi->a({-href => href(action=>"blob", hash=>$diff{'from_id'},
-			                             hash_base=>$parent, file_name=>$diff{'file'})},
-				      "blob") . " | ";
 			print $cgi->a({-href => href(action=>"blame", hash_base=>$parent,
 			                             file_name=>$diff{'file'})},
 			              "blame") . " | ";
@@ -2015,9 +2012,6 @@ sub git_difftree_body {
 				}
 				print " | ";
 			}
-			print $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
-						     hash_base=>$hash, file_name=>$diff{'file'})},
-				      "blob") . " | ";
 			print $cgi->a({-href => href(action=>"blame", hash_base=>$hash,
 			                             file_name=>$diff{'file'})},
 			              "blame") . " | ";
@@ -2058,9 +2052,6 @@ sub git_difftree_body {
 				}
 				print " | ";
 			}
-			print $cgi->a({-href => href(action=>"blob", hash=>$diff{'from_id'},
-						     hash_base=>$parent, file_name=>$diff{'from_file'})},
-				      "blob") . " | ";
 			print $cgi->a({-href => href(action=>"blame", hash_base=>$parent,
 			                             file_name=>$diff{'from_file'})},
 			              "blame") . " | ";
-- 
1.4.3.3

^ permalink raw reply related

* git fetch with multiple remotes failing?
From: Michael S. Tsirkin @ 2006-10-31 15:59 UTC (permalink / raw)
  Cc: git


$ cat .git/remotes/origin
URL: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
Pull: +refs/heads/master:refs/heads/linus_master
$ cat .git/remotes/jejb-scsi-misc-2.6
URL: git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi-misc-2.6.git
Pull: +refs/heads/master:refs/heads/jejb-scsi-misc-2.6
$ git fetch -f origin jejb-scsi-misc-2.6
error: no such remote ref refs/heads/jejb-scsi-misc-2.6
Fetch failure:
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git

Looks I must give remotes one by one?


-- 

^ permalink raw reply

* Re: [PATCH] Introduce git-mirror, a tool for exactly mirroring another repository.
From: Shawn Pearce @ 2006-10-31 14:55 UTC (permalink / raw)
  To: Sergey Vlasov; +Cc: Petr Baudis, Junio C Hamano, git
In-Reply-To: <20061031174225.3c7c1e77.vsu@altlinux.ru>

Sergey Vlasov <vsu@altlinux.ru> wrote:
> On Mon, 25 Sep 2006 00:46:41 -0400 Shawn Pearce wrote:
> 
> > Sometimes its handy to be able to efficiently backup or mirror one
> > Git repository to another Git repository by employing the native
> > Git object transfer protocol.  But when mirroring or backing up a
> > repository you really want:
> >
> >   1) Every object in the source to go to the mirror.
> >   2) Every ref in the source to go to the mirror.
> >   3) Any ref removed from the source to be removed from the mirror.
> >   4) Automatically repack and prune the mirror when necessary.
> >
> > and since git-fetch doesn't do 2, 3, and 4 here's a tool that does.
> 
> Is this patch forgotten, abandoned or what?

Its waiting around for someone to clean it up.  :-)

Junio didn't accept it as there were a number of issues that he
identified in the patch.  I still have them in my inbox but have
not had time to go back and fix them.  Since I haven't actually had
a real need for git-mirror it has been low on my priority list of
"git things to do".

Your message pointed out a number of issues with the current version
that would be worth fixing before accepting it into git.git.  I agree
with many of them, especially about the direct ref manipulation.
git-mirror was written before the Linus packed ref stuff came
along, so git-update-ref didn't have a -d option at the time...
otherwise I would have used it.

-- 

^ permalink raw reply

* Problem with git-push
From: Florent Thoumie @ 2006-10-31 14:48 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 1726 bytes --]

Hi list,

I'm having some weird issues with git-push lately (1.4.1 on both ends):

: flz@cream:/tinderbox/portstrees/xorg-modular/ports; git push
updating 'refs/heads/xorg'
  from 6d1e4fce0a22da799175ec8e168dc0767f1131ef
  to   496cbd4c75220c204aac4fd1ef9912e40e0314b2
Generating pack...
Done counting 16 objects.
Result has 11 objects.
Deltifying 11 objects.
 100% (11/11) done
Total 11, written 11 (delta 4), reused 0 (delta 0)
Unpacking 11 objects
unpack unpacker exited with error code
ng refs/heads/xorg n/a (unpacker error)
unable to write sha1 filename ./objects/2d: Is a directory
fatal: failed to write object
: flz@cream:/tinderbox/portstrees/xorg-modular/ports; git push
updating 'refs/heads/xorg'
  from 6d1e4fce0a22da799175ec8e168dc0767f1131ef
  to   496cbd4c75220c204aac4fd1ef9912e40e0314b2
Generating pack...
Done counting 16 objects.
Result has 11 objects.
Deltifying 11 objects.
 100% (11/11) done
Total 11, written 11 (delta 4), reused 0 (delta 0)
Unpacking 11 objects
unpack unpacker exited with error code
ng refs/heads/xorg n/a (unpacker error)
unable to write sha1 filename ./objects/d7: Is a directory
fatal: failed to write object
: flz@cream:/tinderbox/portstrees/xorg-modular/ports; git push
updating 'refs/heads/xorg'
  from 6d1e4fce0a22da799175ec8e168dc0767f1131ef
  to   496cbd4c75220c204aac4fd1ef9912e40e0314b2
Generating pack...
Done counting 16 objects.
Result has 11 objects.
Deltifying 11 objects.
 100% (11/11) done
Total 11, written 11 (delta 4), reused 0 (delta 0)
Unpacking 11 objects
refs/heads/xorg: 6d1e4fce0a22da799175ec8e168dc0767f1131ef ->
496cbd4c75220c204aac4fd1ef9912e40e0314b2

Any idea what that could be?

Cheers.

Florent

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 187 bytes --]

^ permalink raw reply

* Re: [PATCH] Introduce git-mirror, a tool for exactly mirroring another repository.
From: Sergey Vlasov @ 2006-10-31 14:42 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Petr Baudis, Junio C Hamano, git
In-Reply-To: <20060925044641.GB15757@spearce.org>

[-- Attachment #1: Type: text/plain, Size: 6212 bytes --]

On Mon, 25 Sep 2006 00:46:41 -0400 Shawn Pearce wrote:

> Sometimes its handy to be able to efficiently backup or mirror one
> Git repository to another Git repository by employing the native
> Git object transfer protocol.  But when mirroring or backing up a
> repository you really want:
>
>   1) Every object in the source to go to the mirror.
>   2) Every ref in the source to go to the mirror.
>   3) Any ref removed from the source to be removed from the mirror.
>   4) Automatically repack and prune the mirror when necessary.
>
> and since git-fetch doesn't do 2, 3, and 4 here's a tool that does.

Is this patch forgotten, abandoned or what?

[...]
> --- /dev/null
> +++ b/git-mirror.perl
> @@ -0,0 +1,111 @@
> +#!/usr/bin/env perl
> +# Copyright (C) 2006, Shawn Pearce <spearce@spearce.org>
> +# This file is licensed under the GPL v2, or a later version
> +# at the discretion of Linus.
> +
> +use warnings;
> +use strict;
> +use Git;
> +
> +sub ls_refs ($$);
> +
> +my $remote = shift || 'origin';
> +my $repo = Git->repository();
> +
> +# Verify its OK to execute in this repository.
> +#
> +my $mirror_ok = $repo->config('mirror.allowed') || 0;
> +unless ($mirror_ok =~ /^(?:true|t|yes|y|1)$/i) {
> +	print STDERR <<EOF;
> +error: mirror.allowed is false.
> +error:
> +error: For safety reasons please set mirror.allowed in this repository's
> +error: config before using this command.
> +error:
> +error: Unless you are using this repository ONLY for mirroring another
> +error: repository you probably don't want to do this.
> +EOF
> +	exit 1;
> +}
> +
> +# Build our list of refs.
> +#
> +my $remote_refs = ls_refs($repo, $remote);
> +my $local_refs = ls_refs($repo, $repo->repo_path());
> +my $remote_HEAD = $remote_refs->{'HEAD'};
> +delete $remote_refs->{'HEAD'};
> +delete $local_refs->{'HEAD'};
> +
> +# Delete any local refs which the server no longer contains.
> +#
> +foreach my $ref (keys %$local_refs) {
> +	next if $remote_refs->{$ref};
> +	print "removing $ref\n";
> +	my $log = "logs/$ref";
> +	unlink($repo->repo_path() . '/' . $ref);
> +	unlink($repo->repo_path() . '/' . $log);
> +	rmdir($repo->repo_path() . '/' . $ref) while $ref =~ s,/[^/]*$,,;
> +	rmdir($repo->repo_path() . '/' . $log) while $log =~ s,/[^/]*$,,;

One more instance of direct ref manipulation...

What is the current state of packed-refs implementation?  Would it be
possible to merge at least the interface part (git-update-ref -d for
deleting refs) while keeping the current ref representation, so that
other programs could use proper interfaces now?

> +}

It would be better to remove old refs after the fetch, because if
someone has renamed a branch, and you remove the old ref before the
fetch, you will fetch all commits for this branch again, even if they
were already available under a different name.  However, this may create
problems during git-fetch due to file/directory conflicts.

Another option is to rename deleted refs to something which does not
collide with other refs - git-fetch will see them irrespective of the
name.

Yet another option is to select temporary names for all fetched refs to
be passed to git-fetch, then read their SHA-1 values after git-fetch
(they might differ from the previous result of git-ls-remote), update
normal refs and delete temporary refs.

BTW, I was thinking about the possibility to save removed refs under,
e.g., refs/old/`date -I`/; maybe even non-fast-forward refs could be
saved there - this will ensure that no object will ever disappear from
the mirror, no matter what is done on the master side.  Obviously, in
this case remote refs like refs/old/* should be filtered.

> +
> +# Execute the fetch for any refs which differ from our own.
> +# We don't worry about trying to optimize for rewinds or
> +# exact branch copies as they are rather uncommon.
> +#
> +my @to_fetch;
> +while (my ($ref, $hash) = each %$remote_refs) {
> +	push(@to_fetch, "$ref:$ref")
> +		if (!$local_refs->{$ref} || $local_refs->{$ref} ne $hash);
> +}
> +if (@to_fetch) {
> +	git_cmd_try {
> +		$repo->command_noisy('fetch',
> +			'--force',
> +			'--update-head-ok',
> +			$remote, sort @to_fetch);

This will fail for repositories with large number of refs due to the
limit on command line size.

I'm not sure how to figure out the limit for splitting this to multiple
git-fetch calls.  And using xargs fed from a pipe means that we cannot
use existing Git.pm interfaces.

> +	} '%s failed w/ code %d';
> +} else {
> +	print "No changed refs.  Skipping fetch.\n";
> +}
> +
> +# See what the remote has HEAD pointing at and update our local
> +# HEAD to point at the any ref which points at the same hash.
> +#
> +my %by_hash = map {$remote_refs->{$_} => $_}
> +	grep {m,^refs/heads/,}
> +	keys %$remote_refs;
> +my $HEAD = $by_hash{$remote_HEAD} || 'refs/heads/master';
> +print "Setting HEAD to $HEAD\n";
> +print "                ($remote_HEAD)\n";
> +git_cmd_try {
> +	$repo->command_noisy('symbolic-ref', 'HEAD', $HEAD);
> +} '%s failed w/ code %d';
> +
> +# Repack if we have a large number of loose objects.
> +#
> +if (@to_fetch) {
> +	my $count_output = $repo->command('count-objects');
> +	my ($cur_loose) = ($count_output =~ /^(\d+) objects/);
> +	my $max_loose = $repo->config('mirror.maxlooseobjects') || 100;
> +	if ($cur_loose >= $max_loose) {
> +		git_cmd_try {
> +			$repo->command_noisy('repack', '-a', '-d');
> +			$repo->command_noisy('prune');

git-prune might be unsafe when used concurrently with other
operations...  This is probably not a concern for a mirror repository,
if something ensures that two mirroring processes for the same
repository cannot be started.

> +		} '%s failed w/ code %d';
> +	}
> +}
> +
> +sub ls_refs ($$) {
> +	my $repo = shift;
> +	my $name = shift;
> +	my ($fh, $c) = $repo->command_output_pipe('ls-remote', $name);
> +	my %refs;
> +	while (<$fh>) {
> +		chomp;
> +		next if /\^{}$/;
> +		my ($hash, $ref) = split(/\t/, $_, 2);
> +		$refs{$ref} = $hash if ($ref eq 'HEAD' || $ref =~ m,^refs/,);

Probably more sanity checks would be good here (to make sure that the
other end does not feed you bogus ref names or hashes).

> +	}
> +	$repo->command_close_pipe($fh, $c);
> +	\%refs;
> +}
> --
> 1.4.2.1.gde2b2-dirty
>

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH 5/n] [take 3] gitweb: New improved patchset view
From: Jakub Narebski @ 2006-10-31 14:22 UTC (permalink / raw)
  To: git
In-Reply-To: <200610301953.01875.jnareb@gmail.com>

Replace "gitweb diff header" with its full sha1 of blobs and replace
it by "git diff" header and extended diff header. Change also somewhat
highlighting of diffs.

Added `file_type_long' subroutine to convert file mode in octal to
file type description (only for file modes which used by git).

Changes:
* "gitweb diff header" which looked for example like below:
    file:_<sha1 before>_ -> file:_<sha1 after>_
  where 'file' is file type and '<sha1>' is full sha1 of blob is
  changed to
    diff --git _a/<file before>_ _b/<file after>_
  In both cases links are visible and use default link style. If file
  is added, a/<file> is not hyperlinked, if file is deleted, b/<file>
  is not hyperlinked.
* there is added "extended diff header", with <path> and <hash>
  hyperlinked (and <hash> shortened to 7 characters), and <mode>
  explained: '<mode>' is extended to '<mode> (<file type description>)',
  where added text is slightly lighter to easy distinguish that it
  was added (and it is difference from git-diff output).
* from-file/to-file two-line header lines have slightly darker color
  than removed/added lines.
* chunk header has now delicate line above for easier finding chunk
  boundary, and top margin of 2px, both barely visible.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
It still does not output empty patches. This is to be adressed
in later patch.

As usual, you can see it at work at my web page
  http://roke (.) dyndns (.) info/cgi-bin/gitweb/gitweb.cgi

Comments?

 gitweb/gitweb.css  |   66 +++++++++++++++----
 gitweb/gitweb.perl |  187 
++++++++++++++++++++++++++++++++++------------------
 2 files changed, 176 insertions(+), 77 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 0eda982..87dbc52 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -291,40 +291,82 @@ td.mode {
 	font-family: monospace;
 }
 
-div.diff a.list {
+/* styling of diffs (patchsets): commitdiff and blobdiff views */
+div.diff.header,
+div.diff.extended_header {
+	white-space: normal;
+}
+
+div.diff.header {
+	font-weight: bold;
+
+	background-color: #edece6;
+
+	margin-top: 4px;
+	padding: 4px 0px 2px 0px;
+	border: solid #d9d8d1;
+	border-width: 1px 0px 1px 0px;
+}
+
+div.diff.header a.path {
+	text-decoration: underline;
+}
+
+div.diff.extended_header,
+div.diff.extended_header a.path,
+div.diff.extended_header a.hash {
+	color: #777777;
+}
+
+div.diff.extended_header .info {
+	color: #b0b0b0;
+}
+
+div.diff.extended_header {
+	background-color: #f6f5ee;
+	padding: 2px 0px 2px 0px;
+}
+
+div.diff a.path,
+div.diff a.hash {
 	text-decoration: none;
 }
 
-div.diff a.list:hover {
+div.diff a.path:hover,
+div.diff a.hash:hover {
 	text-decoration: underline;
 }
 
-div.diff.to_file a.list,
-div.diff.to_file,
+div.diff.to_file a.path,
+div.diff.to_file {
+	color: #007000;
+}
+
 div.diff.add {
 	color: #008800;
 }
 
-div.diff.from_file a.list,
-div.diff.from_file,
+div.diff.from_file a.path,
+div.diff.from_file {
+	color: #aa0000;
+}
+
 div.diff.rem {
 	color: #cc0000;
 }
 
 div.diff.chunk_header {
 	color: #990099;
+
+	border: dotted #ffe0ff;
+	border-width: 1px 0px 0px 0px;
+	margin-top: 2px;
 }
 
 div.diff.incomplete {
 	color: #cccccc;
 }
 
-div.diff_info {
-	font-family: monospace;
-	color: #000099;
-	background-color: #edece6;
-	font-style: italic;
-}
 
 div.index_include {
 	border: solid #d9d8d1;
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4554067..c22fcc5 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -732,6 +732,32 @@ sub file_type {
 	}
 }
 
+# convert file mode in octal to file type description string
+sub file_type_long {
+	my $mode = shift;
+
+	if ($mode !~ m/^[0-7]+$/) {
+		return $mode;
+	} else {
+		$mode = oct $mode;
+	}
+
+	if (S_ISDIR($mode & S_IFMT)) {
+		return "directory";
+	} elsif (S_ISLNK($mode)) {
+		return "symlink";
+	} elsif (S_ISREG($mode)) {
+		if ($mode & S_IXUSR) {
+			return "executable";
+		} else {
+			return "file";
+		};
+	} else {
+		return "unknown";
+	}
+}
+
+
 ## 
----------------------------------------------------------------------
 ## functions returning short HTML fragments, or transforming HTML 
fragments
 ## which don't beling to other sections
@@ -2055,7 +2081,9 @@ sub git_patchset_body {
 	my $patch_idx = 0;
 	my $in_header = 0;
 	my $patch_found = 0;
+	my $skip_patch = 0;
 	my $diffinfo;
+	my (%from, %to);
 
 	print "<div class=\"patchset\">\n";
 
@@ -2065,6 +2093,8 @@ sub git_patchset_body {
 
 		if ($patch_line =~ m/^diff /) { # "git diff" header
 			# beginning of patch (in patchset)
+			$skip_patch = 0;
+
 			if ($patch_found) {
 				# close previous patch
 				print "</div>\n"; # class="patch"
@@ -2074,96 +2104,123 @@ sub git_patchset_body {
 			}
 			print "<div class=\"patch\" id=\"patch". ($patch_idx+1) ."\">\n";
 
+			# read and prepare patch information
 			if (ref($difftree->[$patch_idx]) eq "HASH") {
+				# pre-parsed (or generated by hand)
 				$diffinfo = $difftree->[$patch_idx];
 			} else {
 				$diffinfo = parse_difftree_raw_line($difftree->[$patch_idx]);
 			}
+			$from{'file'} = $diffinfo->{'from_file'} || $diffinfo->{'file'};
+			$to{'file'}   = $diffinfo->{'to_file'}   || $diffinfo->{'file'};
+			if ($diffinfo->{'status'} ne "A") { # not new (added) file
+				$from{'href'} = href(action=>"blob", hash_base=>$hash_parent,
+				                     hash=>$diffinfo->{'from_id'},
+				                     file_name=>$from{'file'});
+			}
+			if ($diffinfo->{'status'} ne "D") { # not deleted file
+				$to{'href'} = href(action=>"blob", hash_base=>$hash,
+				                   hash=>$diffinfo->{'to_id'},
+				                   file_name=>$to{'file'});
+			}
 			$patch_idx++;
 
-			# for now, no extended header, hence we skip empty patches
-			# companion to	next LINE if $in_header;
+			# for now we skip empty patches
 			if ($diffinfo->{'from_id'} eq $diffinfo->{'to_id'}) { # no change
 				$in_header = 1;
+				$skip_patch = 0;
 				next LINE;
 			}
 
-			if ($diffinfo->{'status'} eq "A") { # added
-				print "<div class=\"diff_info\">" . 
file_type($diffinfo->{'to_mode'}) . ":" .
-				      $cgi->a({-href => href(action=>"blob", hash_base=>$hash,
-				                             hash=>$diffinfo->{'to_id'}, 
file_name=>$diffinfo->{'file'})},
-				              $diffinfo->{'to_id'}) . " (new)" .
-				      "</div>\n"; # class="diff_info"
-
-			} elsif ($diffinfo->{'status'} eq "D") { # deleted
-				print "<div class=\"diff_info\">" . 
file_type($diffinfo->{'from_mode'}) . ":" .
-				      $cgi->a({-href => href(action=>"blob", 
hash_base=>$hash_parent,
-				                             hash=>$diffinfo->{'from_id'}, 
file_name=>$diffinfo->{'file'})},
-				              $diffinfo->{'from_id'}) . " (deleted)" .
-				      "</div>\n"; # class="diff_info"
-
-			} elsif ($diffinfo->{'status'} eq "R" || # renamed
-			         $diffinfo->{'status'} eq "C" || # copied
-			         $diffinfo->{'status'} eq "2") { # with two filenames (from 
git_blobdiff)
-				print "<div class=\"diff_info\">" .
-				      file_type($diffinfo->{'from_mode'}) . ":" .
-				      $cgi->a({-href => href(action=>"blob", 
hash_base=>$hash_parent,
-				                             hash=>$diffinfo->{'from_id'}, 
file_name=>$diffinfo->{'from_file'})},
-				              $diffinfo->{'from_id'}) .
-				      " -> " .
-				      file_type($diffinfo->{'to_mode'}) . ":" .
-				      $cgi->a({-href => href(action=>"blob", hash_base=>$hash,
-				                             hash=>$diffinfo->{'to_id'}, 
file_name=>$diffinfo->{'to_file'})},
-				              $diffinfo->{'to_id'});
-				print "</div>\n"; # class="diff_info"
-
-			} else { # modified, mode changed, ...
-				print "<div class=\"diff_info\">" .
-				      file_type($diffinfo->{'from_mode'}) . ":" .
-				      $cgi->a({-href => href(action=>"blob", 
hash_base=>$hash_parent,
-				                             hash=>$diffinfo->{'from_id'}, 
file_name=>$diffinfo->{'file'})},
-				              $diffinfo->{'from_id'}) .
-				      " -> " .
-				      file_type($diffinfo->{'to_mode'}) . ":" .
-				      $cgi->a({-href => href(action=>"blob", hash_base=>$hash,
-				                             hash=>$diffinfo->{'to_id'}, 
file_name=>$diffinfo->{'file'})},
-				              $diffinfo->{'to_id'});
-				print "</div>\n"; # class="diff_info"
+			# print "git diff" header
+			$patch_line =~ s!^(diff (.*?) )a/.*$!$1!;
+			if ($from{'href'}) {
+				$patch_line .= $cgi->a({-href => $from{'href'}, -class => "path"},
+				                       'a/' . esc_path($from{'file'}));
+			} else { # file was added
+				$patch_line .= 'a/' . esc_path($from{'file'});
+			}
+			$patch_line .= ' ';
+			if ($to{'href'}) {
+				$patch_line .= $cgi->a({-href => $to{'href'}, -class => "path"},
+				                       'b/' . esc_path($to{'file'}));
+			} else { # file was deleted
+				$patch_line .= 'b/' . esc_path($to{'file'});
 			}
 
-			#print "<div class=\"diff extended_header\">\n";
+			print "<div class=\"diff header\">$patch_line</div>\n";
+			print "<div class=\"diff extended_header\">\n";
 			$in_header = 1;
 			next LINE;
+		} else {
+			# actual skipping of empty patches
+			next LINE if $skip_patch;
 		} # start of patch in patchset
 
+		if ($in_header) {
+			if ($patch_line !~ m/^---/) {
+				# match <path>
+				if ($patch_line =~ s!^((copy|rename) from ).*$!$1! && 
$from{'href'}) {
+					$patch_line .= $cgi->a({-href=>$from{'href'}, -class=>"path"},
+					                        esc_path($from{'file'}));
+				}
+				if ($patch_line =~ s!^((copy|rename) to ).*$!$1! && $to{'href'}) {
+					$patch_line = $cgi->a({-href=>$to{'href'}, -class=>"path"},
+					                      esc_path($to{'file'}));
+				}
+				# match <mode>
+				if ($patch_line =~ m/\s(\d{6})$/) {
+					$patch_line .= '<span class="info"> (' .
+					               file_type_long($1) .
+					               ')</span>';
+				}
+				# match <hash>
+				if ($patch_line =~ m/^index/) {
+					my ($from_link, $to_link);
+					if ($from{'href'}) {
+						$from_link = $cgi->a({-href=>$from{'href'}, -class=>"hash"},
+						                     substr($diffinfo->{'from_id'},0,7));
+					} else {
+						$from_link = '0' x 7;
+					}
+					if ($to{'href'}) {
+						$to_link = $cgi->a({-href=>$to{'href'}, -class=>"hash"},
+						                   substr($diffinfo->{'to_id'},0,7));
+					} else {
+						$to_link = '0' x 7;
+					}
+					my ($from_id, $to_id) = ($diffinfo->{'from_id'}, 
$diffinfo->{'to_id'});
+					$patch_line =~ s!$from_id\.\.$to_id!$from_link..$to_link!;
+				}
+				print $patch_line . "<br/>\n";
 
-		if ($in_header && $patch_line =~ m/^---/) {
-			#print "</div>\n"; # class="diff extended_header"
-			$in_header = 0;
+			} else {
+				#$in_header && $patch_line =~ m/^---/;
+				print "</div>\n"; # class="diff extended_header"
+				$in_header = 0;
+
+				if ($from{'href'}) {
+					$patch_line = '--- a/' .
+					              $cgi->a({-href=>$from{'href'}, -class=>"path"},
+					                      esc_path($from{'file'}));
+				}
+				print "<div class=\"diff from_file\">$patch_line</div>\n";
 
-			my $file = $diffinfo->{'from_file'};
-			$file  ||= $diffinfo->{'file'};
-			$file = $cgi->a({-href => href(action=>"blob", 
hash_base=>$hash_parent,
-			                               hash=>$diffinfo->{'from_id'}, 
file_name=>$file),
-			                -class => "list"}, esc_path($file));
-			$patch_line =~ s|a/.*$|a/$file|g;
-			print "<div class=\"diff from_file\">$patch_line</div>\n";
+				$patch_line = <$fd>;
+				chomp $patch_line;
 
-			$patch_line = <$fd>;
-			chomp $patch_line;
+				#$patch_line =~ m/^+++/;
+				if ($to{'href'}) {
+					$patch_line = '+++ b/' .
+					              $cgi->a({-href=>$to{'href'}, -class=>"path"},
+					                      esc_html($to{'file'}));
+				}
+				print "<div class=\"diff to_file\">$patch_line</div>\n";
 
-			#$patch_line =~ m/^+++/;
-			$file    = $diffinfo->{'to_file'};
-			$file  ||= $diffinfo->{'file'};
-			$file = $cgi->a({-href => href(action=>"blob", hash_base=>$hash,
-			                               hash=>$diffinfo->{'to_id'}, 
file_name=>$file),
-			                -class => "list"}, esc_path($file));
-			$patch_line =~ s|b/.*|b/$file|g;
-			print "<div class=\"diff to_file\">$patch_line</div>\n";
+			}
 
 			next LINE;
 		}
-		next LINE if $in_header;
 
 		print format_diff_line($patch_line);
 	}
-- 
1.4.3.3

^ permalink raw reply related

* emacs mode nitpicks
From: Han-Wen Nienhuys @ 2006-10-31 12:58 UTC (permalink / raw)
  To: git; +Cc: julliard

Hello,

I'm getting used to a GIT working environment, and part of that is the 
emacs mode.

I have a couple of nitpicks:

  - the "o" (open in other window, also available in 
cvs-mode/darcsum/bufferlist) doesn't work in git-status

  - it would be cool to have the return key function like in darcsum.

In darcsum, "enter" toggles showing of a diff of said file. When the 
diff is shown, the file is automatically marked for inclusion in the 
commit. Inside the diff, I can jump to the exact line with C-c.

In darcsum, I can select files for commit using only enter and arrow 
keys. In git-mode, I need the 'm', 'u', '=' keys.

(darcsum is at http://chneukirchen.org/repos/darcsum/ ; while Darcs does 
it have its issues, the Emacs mode is really very neat.)

  - When pressing 'c' for commit, the my cursor is at the "Author:" 
line. Moving to end of buffer  is just a single extra keystroke,  but 
it's completely superfluous and therefore: highly irritating keystroke.

I tried adding a (end-of-buffer) to the function definition, but it 
didn't work, and I'm not enough of an elisp guru to understand why it 
doesn't work.

-- 
  Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen

^ permalink raw reply

* Re: git_get_projects_list and $projects_list
From: Jakub Narebski @ 2006-10-31 12:23 UTC (permalink / raw)
  To: git
In-Reply-To: <ei7dq4$828$1@sea.gmane.org>

Jakub Narebski wrote:

> Aneesh Kumar wrote:
> 
>> Ok if i have
>> 
>> $projects_list = "/a/git////" ==> ending "/" . the function will fail
>> at check_export_ok()
>> 
>> That is because we get the $pfxlen wrong. We should ignore all the trailing "/"
>> my $subdir = substr($File::Find::name, $pfxlen + 1);
> 
> Perhaps it would be enough to just add 
>         $dir =~ s!/+$//; 

Ooops, it should of course be

        $dir =~ s!/+$!!;

> before  
>         my $pfxlen = length("$dir");
> (or something equivalent). Try it and send us a patch.


-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git


^ permalink raw reply

* Re: git_get_projects_list and $projects_list
From: Jakub Narebski @ 2006-10-31 11:59 UTC (permalink / raw)
  To: git
In-Reply-To: <cc723f590610310347h58cdd69bse6d96b19479a4f6a@mail.gmail.com>

Aneesh Kumar wrote:

> Ok if i have
> 
> $projects_list = "/a/git////" ==> ending "/" . the function will fail
> at check_export_ok()
> 
> That is because we get the $pfxlen wrong. We should ignore all the trailing "/"
> my $subdir = substr($File::Find::name, $pfxlen + 1);

Perhaps it would be enough to just add 
        $dir =~ s!/+$//; 
before  
        my $pfxlen = length("$dir");
(or something equivalent). Try it and send us a patch.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git


^ permalink raw reply

* git_get_projects_list and $projects_list
From: Aneesh Kumar @ 2006-10-31 11:47 UTC (permalink / raw)
  To: Git Mailing List

Ok if i have

$projects_list = "/a/git////" ==> ending "/" . the function will fail
at check_export_ok()


That is because we get the $pfxlen wrong. We should ignore all the trailing "/"
my $subdir = substr($File::Find::name, $pfxlen + 1);



^ permalink raw reply

* Re: [PATCH 1/5] upload-pack: no longer call rev-list
From: Johannes Schindelin @ 2006-10-31 10:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0610311054500.26682@wbgn013.biozentrum.uni-wuerzburg.de>

Hi,

On Tue, 31 Oct 2006, Johannes Schindelin wrote:

> Except show_commit() should not show "commit " in front of each line. 
> So, another parameter?

Sorry, that was not clear. When using traverse_commit_list() from 
upload-pack, the "commit " prefix is _not_ shown. With every other 
user of travsere_commit_list(), it is.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 1/5] upload-pack: no longer call rev-list
From: Johannes Schindelin @ 2006-10-31  9:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v1woprrwi.fsf@assigned-by-dhcp.cox.net>



On Mon, 30 Oct 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > It is trivial to do now,...
> 
> May be, but can we do something about these duplicated code?
> 
> > @@ -57,6 +60,40 @@ static ssize_t send_client_data(int fd,
> >  	return safe_write(fd, data, sz);
> >  }
> >  
> > +FILE *pack_pipe = NULL;
> > +static void show_commit(struct commit *commit)
> > [...]

Yes, we could make mark_edges_uninteresting() and traverse_commit_list() 
take a FILE * parameter, which is then passed to the functions. Hmmm?

Except show_commit() should not show "commit " in front of each line. So, 
another parameter?

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 1/2] Allow '-' in config variable names
From: Johannes Schindelin @ 2006-10-31  9:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7vfyd5rxvg.fsf@assigned-by-dhcp.cox.net>

Hi,

On Mon, 30 Oct 2006, Junio C Hamano wrote:

> Junio C Hamano <junkio@cox.net> writes:
> 
> > Linus Torvalds <torvalds@osdl.org> writes:
> >
> >> I need this in order to allow aliases of the same form as "ls-tree", 
> >> "rev-parse" etc, so that I can use
> >>
> >> 	[alias]
> >> 		my-cat=--paginate cat-file -p
> >>
> >> to add a "git my-cat" command.
> >
> Seconds?

I personally dislike any name with "_" or "-" in it, since I am stuck with 
different keyboard layouts and keep mistyping them. I even often find 
myself hitting <TAB> just to complete a _single_ "-", since the position 
of the <TAB> key is not wildly jumping around between different keyboard 
layouts.

So I can live without it.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 2/n] gitweb: Use '&iquot;' instead of '?' in esc_path
From: Jakub Narebski @ 2006-10-31  9:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvem1s29g.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
>
> Junio C Hamano <junkio@cox.net> writes:
> 
>> Jakub Narebski <jnareb@gmail.com> writes:
>>
>>> Use "&iquot;" Latin 1 entity ("&#191;" -- inverted question mark =
>>> turned question mark, U+00BF ISOnum) instead '?' as replacements for
>>> control characters and other undisplayable characters.
>>>
>>> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
>>
>> Do you have something against our Spanish and Latin American
>> friends?  ;-)
>>
>> I wonder if there is a more suitable replacement character that
>> is accepted across scripts?
> 
> I have a suspicion that instead of finding an exotic character,
> just showing the byte value in \octal, perhaps in different
> color, might be more portable and easier.  For one thing, it
> helps to show the exact byte value than just one substitution
> character if you are troubleshooting gitweb.

Will do. Well, with the exception of control characters which have
literal escape characters, for example showing LF as \n instead
of \octal. By the way, wouldn't \xhex be better?

Different color/style is good. Otherwise we would have escape escape
character, i.e. write '\\' and perhaps als mark filename as quoted.
git surrounds filename with doublequotes, "<quoted filename>", adding
'"' to the list of quoted characters: for gitweb this kind of marking
filename as quoted is PITA, as we first, don't want I think to include
'"' in link ("_filename_" vs _"filename"_), and second and more important
if we hyperlink parts of filename we would want for quote to be placed
outside whole filename ("a/_filename_" vs a/_"filename"_ and also
"_path_/_to_/_file_" vs _path_/_to_/_"file"_).

Should we mark only replaced character, or the whole filename?


On the technical side, should I send the patch as replacement for this
patch, or on top of this patch?
-- 
Jakub Narebski

^ permalink raw reply

* [PATCH 2/2] Teach receive-pack how to keep pack files based on object count.
From: Shawn Pearce @ 2006-10-31  7:57 UTC (permalink / raw)
  To: Junio C Hamano, Nicolas Pitre; +Cc: git

Since keeping a pushed pack or exploding it into loose objects
should be a local repository decision this teaches receive-pack
to decide if it should call unpack-objects or index-pack --stdin
--fix-thin based on the setting of receive.unpackLimit and the
number of objects contained in the received pack.

If the number of objects (hdr_entries) in the received pack is
below the value of receive.unpackLimit (which is 5000 by default)
then we unpack-objects as we have in the past.

If the hdr_entries >= receive.unpackLimit then we call index-pack and
ask it to include our pid and hostname in the .keep file to make it
easier to identify why a given pack has been kept in the repository.

Currently this leaves every received pack as a kept pack.  We really
don't want that as received packs will tend to be small.  Instead we
want to delete the .keep file automatically after all refs have
been updated.  That is being left as room for future improvement.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 Documentation/config.txt |   11 +++++++-
 cache.h                  |    1 +
 receive-pack.c           |   66 +++++++++++++++++++++++++++++++++++++++++++--
 sha1_file.c              |    2 +-
 4 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index d9e73da..9d3c71c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -301,7 +301,16 @@ imap::
 	The configuration variables in the 'imap' section are described
 	in gitlink:git-imap-send[1].
 
-receive.denyNonFastforwads::
+receive.unpackLimit::
+	If the number of objects received in a push is below this
+	limit then the objects will be unpacked into loose object
+	files. However if the number of received objects equals or
+	exceeds this limit then the received pack will be stored as
+	a pack, after adding any missing delta bases.  Storing the
+	pack from a push can make the push operation complete faster,
+	especially on slow filesystems.
+
+receive.denyNonFastForwards::
 	If set to true, git-receive-pack will deny a ref update which is
 	not a fast forward. Use this to prevent such an update via a push,
 	even if that push is forced. This configuration variable is
diff --git a/cache.h b/cache.h
index e997a85..6cb7e1d 100644
--- a/cache.h
+++ b/cache.h
@@ -376,6 +376,7 @@ extern struct packed_git *parse_pack_ind
 						char *idx_path);
 
 extern void prepare_packed_git(void);
+extern void reprepare_packed_git(void);
 extern void install_packed_git(struct packed_git *pack);
 
 extern struct packed_git *find_sha1_pack(const unsigned char *sha1, 
diff --git a/receive-pack.c b/receive-pack.c
index 7e154c5..b394833 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "pack.h"
 #include "refs.h"
 #include "pkt-line.h"
 #include "run-command.h"
@@ -7,9 +8,8 @@
 
 static const char receive_pack_usage[] = "git-receive-pack <git-dir>";
 
-static const char *unpacker[] = { "unpack-objects", NULL };
-
 static int deny_non_fast_forwards = 0;
+static int unpack_limit = 5000;
 static int report_status;
 
 static char capabilities[] = "report-status";
@@ -25,6 +25,12 @@ static int receive_pack_config(const cha
 		return 0;
 	}
 
+	if (strcmp(var, "receive.unpacklimit") == 0)
+	{
+		unpack_limit = git_config_int(var, value);
+		return 0;
+	}
+
 	return 0;
 }
 
@@ -227,9 +233,63 @@ static void read_head_info(void)
 	}
 }
 
+static const char *parse_pack_header(struct pack_header *hdr)
+{
+	char *c = (char*)hdr;
+	ssize_t remaining = sizeof(struct pack_header);
+	do {
+		ssize_t r = xread(0, c, remaining);
+		if (r <= 0)
+			return "eof before pack header was fully read";
+		remaining -= r;
+		c += r;
+	} while (remaining > 0);
+	if (hdr->hdr_signature != htonl(PACK_SIGNATURE))
+		return "protocol error (pack signature mismatch detected)";
+	if (!pack_version_ok(hdr->hdr_version))
+		return "protocol error (pack version unsupported)";
+	return NULL;
+}
+
 static const char *unpack()
 {
-	int code = run_command_v_opt(1, unpacker, RUN_GIT_CMD);
+	struct pack_header hdr;
+	const char *hdr_err;
+	char hdr_arg[38];
+	int code;
+
+	hdr_err = parse_pack_header(&hdr);
+	if (hdr_err)
+		return hdr_err;
+	snprintf(hdr_arg, sizeof(hdr_arg), "--pack_header=%u,%u",
+		ntohl(hdr.hdr_version), ntohl(hdr.hdr_entries));
+
+	if (ntohl(hdr.hdr_entries) < unpack_limit) {
+		const char *unpacker[3];
+		unpacker[0] = "unpack-objects";
+		unpacker[1] = hdr_arg;
+		unpacker[2] = NULL;
+		code = run_command_v_opt(1, unpacker, RUN_GIT_CMD);
+	} else {
+		const char *keeper[6];
+		char my_host[255], keep_arg[128 + 255];
+
+		if (gethostname(my_host, sizeof(my_host)))
+			strcpy(my_host, "localhost");
+		snprintf(keep_arg, sizeof(keep_arg),
+			"--keep=receive-pack %i on %s",
+			getpid(), my_host);
+
+		keeper[0] = "index-pack";
+		keeper[1] = "--stdin";
+		keeper[2] = "--fix-thin";
+		keeper[3] = hdr_arg;
+		keeper[4] = keep_arg;
+		keeper[5] = NULL;
+		code = run_command_v_opt(1, keeper, RUN_GIT_CMD);
+		if (!code)
+			reprepare_packed_git();
+	}
 
 	switch (code) {
 	case 0:
diff --git a/sha1_file.c b/sha1_file.c
index 5e6c8b8..7bda2d4 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -663,7 +663,7 @@ void prepare_packed_git(void)
 	prepare_packed_git_run_once = 1;
 }
 
-static void reprepare_packed_git(void)
+void reprepare_packed_git(void)
 {
 	prepare_packed_git_run_once = 0;
 	prepare_packed_git();
-- 
1.4.3.3.g7d63

^ permalink raw reply related

* [PATCH 1/2] Allow pack header preprocessing before unpack-objects/index-pack.
From: Shawn Pearce @ 2006-10-31  7:56 UTC (permalink / raw)
  To: Junio C Hamano, Nicolas Pitre; +Cc: git

Some applications which invoke unpack-objects or index-pack --stdin
may want to examine the pack header to determine the number of
objects contained in the pack and use that value to determine which
executable to invoke to handle the rest of the pack stream.

However if the caller consumes the pack header from the input stream
then its no longer available for unpack-objects or index-pack --stdin,
both of which need the version and object count to process the stream.

This change introduces --pack_header=ver,cnt as a command line option
that the caller can supply to indicate it has already consumed the
pack header and what version and object count were found in that
header.  As this option is only meant for low level applications
such as receive-pack we are not documenting it at this time.

Credit goes to Nicolas Pitre for suggesting this approach on the
git mailing list.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 This series of two patches replaces my last receive-pack (3/3)
 patch.

 builtin-unpack-objects.c |   31 +++++++++++++++++++++++++------
 index-pack.c             |   30 +++++++++++++++++++++++++-----
 2 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index e70a711..a5cfd0b 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -326,11 +326,16 @@ static void unpack_one(unsigned nr, unsi
 	}
 }
 
-static void unpack_all(void)
+static void unpack_all(struct pack_header *hdr)
 {
 	int i;
-	struct pack_header *hdr = fill(sizeof(struct pack_header));
-	unsigned nr_objects = ntohl(hdr->hdr_entries);
+	unsigned nr_objects;
+
+	if (!hdr) {
+		hdr = fill(sizeof(struct pack_header));
+		use(sizeof(struct pack_header));
+	}
+	nr_objects = ntohl(hdr->hdr_entries);
 
 	if (ntohl(hdr->hdr_signature) != PACK_SIGNATURE)
 		die("bad pack file");
@@ -339,7 +344,6 @@ static void unpack_all(void)
 	fprintf(stderr, "Unpacking %d objects\n", nr_objects);
 
 	obj_list = xmalloc(nr_objects * sizeof(*obj_list));
-	use(sizeof(struct pack_header));
 	for (i = 0; i < nr_objects; i++)
 		unpack_one(i, nr_objects);
 	if (delta_list)
@@ -348,7 +352,8 @@ static void unpack_all(void)
 
 int cmd_unpack_objects(int argc, const char **argv, const char *prefix)
 {
-	int i;
+	int i, have_hdr = 0;
+	struct pack_header hdr;
 	unsigned char sha1[20];
 
 	git_config(git_default_config);
@@ -371,6 +376,18 @@ int cmd_unpack_objects(int argc, const c
 				recover = 1;
 				continue;
 			}
+			if (!strncmp(arg, "--pack_header=", 14)) {
+				char *c;
+				hdr.hdr_signature = htonl(PACK_SIGNATURE);
+				hdr.hdr_version = htonl(strtoul(arg + 14, &c, 10));
+				if (*c != ',')
+					die("bad %s", arg);
+				hdr.hdr_entries = htonl(strtoul(c + 1, &c, 10));
+				if (*c)
+					die("bad %s", arg);
+				have_hdr = 1;
+				continue;
+			}
 			usage(unpack_usage);
 		}
 
@@ -378,7 +395,9 @@ int cmd_unpack_objects(int argc, const c
 		usage(unpack_usage);
 	}
 	SHA1_Init(&ctx);
-	unpack_all();
+	if (have_hdr)
+		SHA1_Update(&ctx, &hdr, sizeof(struct pack_header));
+	unpack_all(have_hdr ? &hdr : NULL);
 	SHA1_Update(&ctx, buffer, offset);
 	SHA1_Final(sha1, &ctx);
 	if (hashcmp(fill(20), sha1))
diff --git a/index-pack.c b/index-pack.c
index b37dd78..1911df8 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -161,9 +161,12 @@ static const char *open_pack_file(const
 	return pack_name;
 }
 
-static void parse_pack_header(void)
+static void parse_pack_header(struct pack_header *hdr)
 {
-	struct pack_header *hdr = fill(sizeof(struct pack_header));
+	if (!hdr) {
+		hdr = fill(sizeof(struct pack_header));
+		use(sizeof(struct pack_header));
+	}
 
 	/* Header consistency check */
 	if (hdr->hdr_signature != htonl(PACK_SIGNATURE))
@@ -172,7 +175,6 @@ static void parse_pack_header(void)
 		die("pack version %d unsupported", ntohl(hdr->hdr_version));
 
 	nr_objects = ntohl(hdr->hdr_entries);
-	use(sizeof(struct pack_header));
 }
 
 static void bad_object(unsigned long offset, const char *format,
@@ -822,11 +824,12 @@ static void final(const char *final_pack
 
 int main(int argc, char **argv)
 {
-	int i, fix_thin_pack = 0;
+	int i, fix_thin_pack = 0, have_hdr = 0;
 	const char *curr_pack, *pack_name = NULL;
 	const char *curr_index, *index_name = NULL;
 	const char *keep_name = NULL, *keep_msg = NULL;
 	char *index_name_buf = NULL, *keep_name_buf = NULL;
+	struct pack_header hdr;
 	unsigned char sha1[20];
 
 	for (i = 1; i < argc; i++) {
@@ -841,6 +844,16 @@ int main(int argc, char **argv)
 				keep_msg = "";
 			} else if (!strncmp(arg, "--keep=", 7)) {
 				keep_msg = arg + 7;
+			} else if (!strncmp(arg, "--pack_header=", 14)) {
+				char *c;
+				hdr.hdr_signature = htonl(PACK_SIGNATURE);
+				hdr.hdr_version = htonl(strtoul(arg + 14, &c, 10));
+				if (*c != ',')
+					die("bad %s", arg);
+				hdr.hdr_entries = htonl(strtoul(c + 1, &c, 10));
+				if (*c)
+					die("bad %s", arg);
+				have_hdr = 1;
 			} else if (!strcmp(arg, "-v")) {
 				verbose = 1;
 			} else if (!strcmp(arg, "-o")) {
@@ -861,6 +874,8 @@ int main(int argc, char **argv)
 		usage(index_pack_usage);
 	if (fix_thin_pack && !from_stdin)
 		die("--fix-thin cannot be used without --stdin");
+	if (have_hdr && !from_stdin)
+		die("--pack_header cannot be used without --stdin");
 	if (!index_name && pack_name) {
 		int len = strlen(pack_name);
 		if (!has_extension(pack_name, ".pack"))
@@ -883,7 +898,12 @@ int main(int argc, char **argv)
 	}
 
 	curr_pack = open_pack_file(pack_name);
-	parse_pack_header();
+	if (have_hdr) {
+		if (output_fd >= 0)
+			write_or_die(output_fd, &hdr, sizeof(struct pack_header));
+		SHA1_Update(&input_ctx, &hdr, sizeof(struct pack_header));
+	}
+	parse_pack_header(have_hdr ? &hdr : NULL);
 	objects = xmalloc((nr_objects + 1) * sizeof(struct object_entry));
 	deltas = xmalloc(nr_objects * sizeof(struct delta_entry));
 	if (verbose)
-- 
1.4.3.3.g7d63

^ permalink raw reply related

* Re: [PATCH] Teach receive-pack how to keep pack files when unpacklooseobjects = 0.
From: Shawn Pearce @ 2006-10-31  6:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwt6hq8nu.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> Shawn Pearce <spearce@spearce.org> writes:
> 
> >> Because that approach assumes recieve-pack and unpack-objects
> >> and index-pack are from the same vintage (otherwise your
> >> receive-pack would need to have a way to see if unpack-objects
> >> and index-pack would grok --pack_header argument), we could even
> >> get away without passing the pack version if we wanted to.
> >...
> > BTW I think we do need to pass the pack version in the option.
> 
> We are in agreement; I mentioned "we could ... if we wanted to"
> because I wanted to see if you are paying attention ;-).

Hah.  Trying to keep me on my toes aren't you.  :-)

I almost have the change finished. I'll send it before I go to bed.

-- 

^ permalink raw reply

* Re: [PATCH] Teach receive-pack how to keep pack files when unpacklooseobjects = 0.
From: Junio C Hamano @ 2006-10-31  6:52 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git
In-Reply-To: <20061031063941.GB7375@spearce.org>

Shawn Pearce <spearce@spearce.org> writes:

>> Because that approach assumes recieve-pack and unpack-objects
>> and index-pack are from the same vintage (otherwise your
>> receive-pack would need to have a way to see if unpack-objects
>> and index-pack would grok --pack_header argument), we could even
>> get away without passing the pack version if we wanted to.
>...
> BTW I think we do need to pass the pack version in the option.

We are in agreement; I mentioned "we could ... if we wanted to"
because I wanted to see if you are paying attention ;-).


^ permalink raw reply

* Re: [PATCH] Teach receive-pack how to keep pack files when unpacklooseobjects = 0.
From: Shawn Pearce @ 2006-10-31  6:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nicolas Pitre, git
In-Reply-To: <7v7iyhrsoi.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> Nicolas Pitre <nico@cam.org> writes:
> 
> > Why not just parse the pack header in receive-pack / fetch-pack, and 
> > decide on the first-hand information?  Sure the pack header is then 
> > gone, but then the only thing that is needed is an extra flag to both 
> > unpack-objects and index-pack to tell them that we've already parsed the 
> > pack header and that the pack version is x and the number of objects is 
> > y.  Simply something like --pack_header=x,y.  No protocol extension 
> > needed, no extra rev-list, no reliance on the remote server providing 
> > the needed info.
> 
> I like it.
> 
> Because that approach assumes recieve-pack and unpack-objects
> and index-pack are from the same vintage (otherwise your
> receive-pack would need to have a way to see if unpack-objects
> and index-pack would grok --pack_header argument), we could even
> get away without passing the pack version if we wanted to.

Heh.  On Saturday I almost did exactly what Nico proposes above.
But I thought both of you would find the --pack_header=x,y option
too brittle and would reject the change.

But since all three of us liked the same idea I'll code it up
and resend my receive-pack patch using Nico's suggestion instead.
Hopefully I'll get it out tomorrow.

BTW I think we do need to pass the pack version in the option.
If we ever do increment the pack version its going to be after this
option is introduced so supporting the option does not imply that
the callee is able to parse the pack file without knowing what
version the file is, especially if the callee supports both the
current version and the new version.

-- 

^ 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