Git development
 help / color / mirror / Atom feed
* Re: WARNING: THIS PATCH CAN BREAK YOUR REPO, was Re: [PATCH 2/3] Only repack active packs by skipping over kept packs.
From: Jan Harkes @ 2006-10-30 20:52 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Nicolas Pitre, Junio C Hamano, git
In-Reply-To: <20061030202611.GA5775@spearce.org>

On Mon, Oct 30, 2006 at 03:26:11PM -0500, Shawn Pearce wrote:
> Actually the breakage is easier to reproduce without trashing
> a repository.
> 
> Do the above so you have everything in one pack.  Now use rev-list
> to simulate the object list construction in pack-objects as though
> we were doing a 'git repack -a -d':
> 
>   git-rev-list --objects --all \
>     --unpacked=.git/objects/pack/pack-*.pack \
> 	| wc -l
> 
> gives me 102 (WRONG WRONG WRONG WRONG!!!!!!)

The problem seems to be that as soon as we hit something that is found
in a pack that is not on the ignore list, that object and all it's
parents are marked as uninteresting. So if the kept pack contains a
slice of commits (v1.4.3..v1.4.3.3) the revision walker will only return
the recent stuff (v1.4.3.3..) and drop the older data (..v1.4.3).

The following patch does fix the problem Nicolas reported, but for some
reason I'm still getting only 102 objects (only tags and the commits
they refer to?) with your test.

Jan

----
diff --git a/revision.c b/revision.c
index 280e92b..a69c873 100644
--- a/revision.c
+++ b/revision.c
@@ -418,9 +418,6 @@ static void limit_list(struct rev_info *
 
 		if (revs->max_age != -1 && (commit->date < revs->max_age))
 			obj->flags |= UNINTERESTING;
-		if (revs->unpacked &&
-		    has_sha1_pack(obj->sha1, revs->ignore_packed))
-			obj->flags |= UNINTERESTING;
 		add_parents_to_list(revs, commit, &list);
 		if (obj->flags & UNINTERESTING) {
 			mark_parents_uninteresting(commit);
@@ -1149,17 +1146,18 @@ struct commit *get_revision(struct rev_i
 		 * that we'd otherwise have done in limit_list().
 		 */
 		if (!revs->limited) {
-			if ((revs->unpacked &&
-			     has_sha1_pack(commit->object.sha1,
-					   revs->ignore_packed)) ||
-			    (revs->max_age != -1 &&
-			     (commit->date < revs->max_age)))
+			if (revs->max_age != -1 &&
+			    (commit->date < revs->max_age))
 				continue;
 			add_parents_to_list(revs, commit, &revs->commits);
 		}
 		if (commit->object.flags & SHOWN)
 			continue;
 
+		if (revs->unpacked && has_sha1_pack(commit->object.sha1,
+						    revs->ignore_packed))
+		    continue;
+
 		/* We want to show boundary commits only when their
 		 * children are shown.  When path-limiter is in effect,

^ permalink raw reply related

* Re: WARNING: THIS PATCH CAN BREAK YOUR REPO, was Re: [PATCH 2/3] Only repack active packs by skipping over kept packs.
From: Jan Harkes @ 2006-10-30 21:07 UTC (permalink / raw)
  To: Shawn Pearce, Nicolas Pitre, Junio C Hamano, git
In-Reply-To: <20061030205200.GA20236@delft.aura.cs.cmu.edu>

On Mon, Oct 30, 2006 at 03:52:00PM -0500, Jan Harkes wrote:
> On Mon, Oct 30, 2006 at 03:26:11PM -0500, Shawn Pearce wrote:
> > Do the above so you have everything in one pack.  Now use rev-list
> > to simulate the object list construction in pack-objects as though
> > we were doing a 'git repack -a -d':
> > 
> >   git-rev-list --objects --all \
> >     --unpacked=.git/objects/pack/pack-*.pack \
> > 	| wc -l
> > 
> > gives me 102 (WRONG WRONG WRONG WRONG!!!!!!)
> 
...
> 
> The following patch does fix the problem Nicolas reported, but for some
> reason I'm still getting only 102 objects (only tags and the commits
> they refer to?) with your test.

Seems to be operator error, I guess the shell can't (won't) expand
--unpacked=.git/objects/pack/pack-*.pack and there is no pack named
pack-*.pack, so rev-list will actually find every object in one of the
packs and skip them.

The following works correctly,

    $ git-rev-list --objects --all --unpacked=.git/objects/pack/pack-234f8136e45fb34d118bb346c15267535e80e5f0.pack --unpacked=.git/objects/pack/pack-aceb4c6394c586abaf65d76dd6cf088f50a5b806.pack | wc -l
    28713

    $ ~/git/git/git-rev-list --objects --all | wc -l
    28713

Jan

^ permalink raw reply

* Re: WARNING: THIS PATCH CAN BREAK YOUR REPO, was Re: [PATCH 2/3] Only repack active packs by skipping over kept packs.
From: Shawn Pearce @ 2006-10-30 21:09 UTC (permalink / raw)
  To: Jan Harkes; +Cc: Nicolas Pitre, Junio C Hamano, git
In-Reply-To: <20061030205200.GA20236@delft.aura.cs.cmu.edu>

Jan Harkes <jaharkes@cs.cmu.edu> wrote:
> On Mon, Oct 30, 2006 at 03:26:11PM -0500, Shawn Pearce wrote:
> > Actually the breakage is easier to reproduce without trashing
> > a repository.
> > 
> > Do the above so you have everything in one pack.  Now use rev-list
> > to simulate the object list construction in pack-objects as though
> > we were doing a 'git repack -a -d':
> > 
> >   git-rev-list --objects --all \
> >     --unpacked=.git/objects/pack/pack-*.pack \
> > 	| wc -l
> > 
> > gives me 102 (WRONG WRONG WRONG WRONG!!!!!!)
> 
> The problem seems to be that as soon as we hit something that is found
> in a pack that is not on the ignore list, that object and all it's
> parents are marked as uninteresting. So if the kept pack contains a
> slice of commits (v1.4.3..v1.4.3.3) the revision walker will only return
> the recent stuff (v1.4.3.3..) and drop the older data (..v1.4.3).

Right - I got that far in my own research and then saw your patch
drop into my inbox. :-)
 
> The following patch does fix the problem Nicolas reported, but for some
> reason I'm still getting only 102 objects (only tags and the commits
> they refer to?) with your test.

Ack'd.

Your patch fixes both bugs for me.  The rev-list test I talked about
above is now turning up 31846 objects which is the correct count.
The repack test Nico crafted works correctly too.

-- 

^ permalink raw reply

* [PATCH 3/n] gitweb: Use 's' regexp modifier to secure against filenames with LF
From: Jakub Narebski @ 2006-10-30 21:25 UTC (permalink / raw)
  To: git
In-Reply-To: <200610301953.01875.jnareb@gmail.com>

Use 's' (treat string as single line) regexp modifier in
git_get_hash_by_path (against future changes, probably unnecessary)
and in parse_ls_tree_line (when called with '-z'=>1 option) to secure
against filenames containing newline.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Without this patch filename with LF broke "tree" view.

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index edca27d..0fd1360 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -890,7 +890,7 @@ sub git_get_hash_by_path {
 	close $fd or return undef;
 
 	#'100644 blob 0fa3f3a66fb6a137f6ec2c19351ed4d807070ffa	panic.c'
-	$line =~ m/^([0-9]+) (.+) ([0-9a-fA-F]{40})\t(.+)$/;
+	$line =~ m/^([0-9]+) (.+) ([0-9a-fA-F]{40})\t(.+)$/s;
 	if (defined $type && $type ne $2) {
 		# type doesn't match
 		return undef;
@@ -1305,7 +1305,7 @@ sub parse_ls_tree_line ($;%) {
 	my %res;
 
 	#'100644 blob 0fa3f3a66fb6a137f6ec2c19351ed4d807070ffa	panic.c'
-	$line =~ m/^([0-9]+) (.+) ([0-9a-fA-F]{40})\t(.+)$/;
+	$line =~ m/^([0-9]+) (.+) ([0-9a-fA-F]{40})\t(.+)$/s;
 
 	$res{'mode'} = $1;
 	$res{'type'} = $2;
-- 
1.4.3.3

^ permalink raw reply related

* Use memmove instead of memcpy for overlapping areas
From: Edgar Toernig @ 2006-10-30 21:26 UTC (permalink / raw)
  To: git

There may be more - this is just the result of a quick eye-grep
for memcpy(x, x+i).

diff --git a/imap-send.c b/imap-send.c
index 16804ab..88d635f 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -272,7 +272,7 @@ buffer_gets( buffer_t * b, char **s )
 				n = b->bytes - start;
 
 				if (n)
-					memcpy( b->buf, b->buf + start, n );
+					memmove( b->buf, b->buf + start, n );
 				b->offset -= start;
 				b->bytes = n;
 				start = 0;
diff --git a/index-pack.c b/index-pack.c
index e33f605..a275982 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -61,7 +61,7 @@ static void * fill(int min)
 		die("cannot fill %d bytes", min);
 	if (input_offset) {
 		SHA1_Update(&input_ctx, input_buffer, input_offset);
-		memcpy(input_buffer, input_buffer + input_offset, input_len);
+		memmove(input_buffer, input_buffer + input_offset, input_len);
 		input_offset = 0;
 	}
 	do {


^ permalink raw reply related

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

Add "--" after <commit-ish> or <tree-ish> argument to clearly mark it
as <commit-ish> or <tree-ish> and not pathspec, securing against refs
with the same names as files or directories in [live] repository.

Some wrapping to reduce line length as well.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
I uses branch named 'gitweb/test' to test gitweb against files with
funny characters (like '"', '\', TAB, LF) in filenames. I run gitweb
on "live" (not bare) repository, and there is directory 'gitweb/test'
in it. So I had some parts of gitweb not functioning, and errors in
the web server logs. This patch fixes that issue.

 gitweb/gitweb.perl |   38 +++++++++++++++++++++++---------------
 1 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 0fd1360..4554067 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1141,7 +1141,9 @@ sub parse_commit {
 		@commit_lines = @$commit_text;
 	} else {
 		local $/ = "\0";
-		open my $fd, "-|", git_cmd(), "rev-list", "--header", "--parents", "--max-count=1", $commit_id
+		open my $fd, "-|", git_cmd(), "rev-list",
+			"--header", "--parents", "--max-count=1",
+			$commit_id, "--"
 			or return;
 		@commit_lines = split '\n', <$fd>;
 		close $fd or return;
@@ -2559,7 +2561,7 @@ sub git_summary {
 	}
 
 	open my $fd, "-|", git_cmd(), "rev-list", "--max-count=17",
-		git_get_head_hash($project)
+		git_get_head_hash($project), "--"
 		or die_error(undef, "Open git-rev-list failed");
 	my @revlist = map { chomp; $_ } <$fd>;
 	close $fd;
@@ -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");
@@ -3102,7 +3104,7 @@ sub git_log {
 	my $refs = git_get_references();
 
 	my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
-	open my $fd, "-|", git_cmd(), "rev-list", $limit, $hash
+	open my $fd, "-|", git_cmd(), "rev-list", $limit, $hash, "--"
 		or die_error(undef, "Open git-rev-list failed");
 	my @revlist = map { chomp; $_ } <$fd>;
 	close $fd;
@@ -3160,7 +3162,7 @@ sub git_commit {
 		$parent = "--root";
 	}
 	open my $fd, "-|", git_cmd(), "diff-tree", '-r', "--no-commit-id",
-		@diff_opts, $parent, $hash
+		@diff_opts, $parent, $hash, "--"
 		or die_error(undef, "Open git-diff-tree failed");
 	my @difftree = map { chomp; $_ } <$fd>;
 	close $fd or die_error(undef, "Reading git-diff-tree failed");
@@ -3265,7 +3267,8 @@ sub git_blobdiff {
 	if (defined $hash_base && defined $hash_parent_base) {
 		if (defined $file_name) {
 			# read raw output
-			open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts, $hash_parent_base, $hash_base,
+			open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
+				$hash_parent_base, $hash_base,
 				"--", $file_name
 				or die_error(undef, "Open git-diff-tree failed");
 			@difftree = map { chomp; $_ } <$fd>;
@@ -3279,7 +3282,8 @@ sub git_blobdiff {
 			# try to find filename from $hash
 
 			# read filtered raw output
-			open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts, $hash_parent_base, $hash_base
+			open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
+				$hash_parent_base, $hash_base, "--"
 				or die_error(undef, "Open git-diff-tree failed");
 			@difftree =
 				# ':100644 100644 03b21826... 3b93d5e7... M	ls-files.c'
@@ -3349,7 +3353,8 @@ sub git_blobdiff {
 		}
 
 		# open patch output
-		open $fd, "-|", git_cmd(), "diff", '-p', @diff_opts, $hash_parent, $hash
+		open $fd, "-|", git_cmd(), "diff", '-p', @diff_opts,
+			$hash_parent, $hash, "--"
 			or die_error(undef, "Open git-diff failed");
 	} else  {
 		die_error('404 Not Found', "Missing one of the blob diff parameters")
@@ -3480,8 +3485,8 @@ sub git_commitdiff {
 	my @difftree;
 	if ($format eq 'html') {
 		open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
-			"--no-commit-id",
-			"--patch-with-raw", "--full-index", $hash_parent, $hash
+			"--no-commit-id", "--patch-with-raw", "--full-index",
+			$hash_parent, $hash, "--"
 			or die_error(undef, "Open git-diff-tree failed");
 
 		while (chomp(my $line = <$fd>)) {
@@ -3492,7 +3497,7 @@ sub git_commitdiff {
 
 	} elsif ($format eq 'plain') {
 		open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
-			'-p', $hash_parent, $hash
+			'-p', $hash_parent, $hash, "--"
 			or die_error(undef, "Open git-diff-tree failed");
 
 	} else {
@@ -3669,7 +3674,9 @@ sub git_search {
 	my $alternate = 1;
 	if ($searchtype eq 'commit' or $searchtype eq 'author' or $searchtype eq 'committer') {
 		$/ = "\0";
-		open my $fd, "-|", git_cmd(), "rev-list", "--header", "--parents", $hash or next;
+		open my $fd, "-|", git_cmd(), "rev-list",
+			"--header", "--parents", $hash, "--"
+			or next;
 		while (my $commit_text = <$fd>) {
 			if (!grep m/$searchtext/i, $commit_text) {
 				next;
@@ -3815,7 +3822,7 @@ sub git_shortlog {
 	my $refs = git_get_references();
 
 	my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
-	open my $fd, "-|", git_cmd(), "rev-list", $limit, $hash
+	open my $fd, "-|", git_cmd(), "rev-list", $limit, $hash, "--"
 		or die_error(undef, "Open git-rev-list failed");
 	my @revlist = map { chomp; $_ } <$fd>;
 	close $fd;
@@ -3843,7 +3850,8 @@ sub git_shortlog {
 
 sub git_rss {
 	# http://www.notestips.com/80256B3A007F2692/1/NAMO5P9UPQ
-	open my $fd, "-|", git_cmd(), "rev-list", "--max-count=150", git_get_head_hash($project)
+	open my $fd, "-|", git_cmd(), "rev-list", "--max-count=150",
+		git_get_head_hash($project), "--"
 		or die_error(undef, "Open git-rev-list failed");
 	my @revlist = map { chomp; $_ } <$fd>;
 	close $fd or die_error(undef, "Reading git-rev-list failed");
@@ -3867,7 +3875,7 @@ XML
 		}
 		my %cd = parse_date($co{'committer_epoch'});
 		open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
-			$co{'parent'}, $co{'id'}
+			$co{'parent'}, $co{'id'}, "--"
 			or next;
 		my @difftree = map { chomp; $_ } <$fd>;
 		close $fd
-- 
1.4.3.3

^ permalink raw reply related

* Re: [PATCH/RFC (take 2)] gitweb: New improved patchset view
From: Luben Tuikov @ 2006-10-30 21:34 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200610300905.04454.jnareb@gmail.com>

--- Jakub Narebski <jnareb@gmail.com> wrote:
> > Wouldn't this be confusing with the other fine lines?
> > I personally don't like this chunk separation.  Chunk separation
> > already exists as is and we view it all the time elsewhere.
> 
> But not always the program displaying diff can display such line
> separating chunks, for example on text terminal it can't.

What I meant is that since I stare at diffs exactly on text terminals,
my eyes have found other ways to discern chunk blocks.

> But if you think that the dotted 1px #ffbbff line is too intrusive,
> we can remove it (and perhaps increase vertical space a few pixels).
> I'd like to have more opinions first.

No, I just think that it should be as close as possible to what
we see now and what we see on text terminals -- no extra vertical
space please.  Between the two evils, I'd prefer the thin "dotted" line.

> BTW. you can easily override it in your CSS file.

Why should we allow something to go into gitweb and disrupt the current
default behavior only so that people have to change their own css file
to keep current default behaviour.  Please don't shove this down our
throats.  Please?

   Luben

^ permalink raw reply

* Re: VCS comparison table
From: Jan Hudec @ 2006-10-30 21:46 UTC (permalink / raw)
  To: David Lang; +Cc: Matthew D. Fuller, Linus Torvalds, bazaar-ng, git
In-Reply-To: <Pine.LNX.4.63.0610251459160.1754@qynat.qvtvafvgr.pbz>

On Wed, Oct 25, 2006 at 03:40:00PM -0700, David Lang wrote:
> On Tue, 24 Oct 2006, Matthew D. Fuller wrote:
> >On Tue, Oct 24, 2006 at 11:03:20AM -0700 I heard the voice of
> >David Lang, and lo! it spake thus:
> >>
> >>it sounded like you were saying that the way to get the slices of
> >>the DAG was to use branches in bzr. [...]
> >
> >I'm not entirely sure I understand what you mean here, but I think
> >you're saying "Nobody's written the code in bzr to show arbitrary
> >slices of the DAG", which is true TTBOMK.
> 
> I think we are talking past each other here.
> 
> what I think was said was
> 
> G 'one feature of git is that you can view arbatrary slices trivially'
> 
> B 'bzr can do this too, you just use branches to define the slices'
> 
> G 'but this limits you becouse branches are defined as code is developed, 
> git lets you define slices at viewing time'
> 
> by the way, I think it's more then just saying 'well, the code could be 
> written to do this in $VCS' some decisions and standard ways of doing 
> things can impact how hard it is to implement a feature, and some decisions 
> can make it impossible (without doing unexpected things).

Since bzr branch is, and is ONLY, a pointer to a revision, I don't see
any design decision that would make this harder in bzr. The UI was only
implemented to take the revisions as branches.

> >>everyone agrees that bzr supports the Star topology. Most people
> >>(including bzr people) seem to agree that currently bzr does not
> >>support the Distributed topology.
> >
> >I think this statement arouses so much grumbling because (a) bzr does
> >support such a lot better than often seems implied, (b) where it
> >doesn't, the changes needed to do so are relatively minor (often
> >merely cosmetic), and (c) disagreement over whether some of the
> >qualifications included for 'distributed' are really fundamental.

The more I read this thread I actually think bzr does support
distributed topology as well as git.

The whole difference is that bzr makes a distinction between the first
and other parents of a revision, while git does not. This distinction is
done in two places:

1. The log shows the first parent and than, as indented subsection the
   ancestry of other parents until the point where the ancestries meet
   again. This actually captures a pattern people usually use. When you
   merge, you usually put in the log something along the lines:

   "merged X, which bars and fixes foo."

   when you actually merge M, which you consider a "mainline" and
   therefore not worth mentioning and X. Linus does it this way too --
   he actually posted a log message as an example, that showed exactly
   this.

2. Assigns revision aliases in this same order (except the "major"
   number for the subsection is based on the common ancestor, not on the
   merge point). They are not special thing that is generated at commit
   time; they are infered from the shape of the DAG (and cached for
   performance reasons).

And the only issue I think is, that the bzr UI and documentation pushes
forward these aliases (revnos) more than appropriate for fully
distributed case and hides the real revision names (revids) too much for
that case.

> >>it's just fine for bzr to not support all possible topologies,
> >
> >I think there's a real intent for bzr TO support at least all common
> >topologies.  I'll buy that current development has focused more on
> >[relatively] simple topologies than the more wildly complex ones.  I
> >look forward to more addressing of the less common cases as the tool
> >matures, and I think a lot of this thread will be good material to
> >work with as that happens.  It's just the suggestion that providing
> >fruit for simple topologies _necessarily_ prejudices against complex
> >ones that I find so onerous.
> 
> one concern that the git people are voicing is that the things that work 
> for simple topologies (revno's) can't be used with the more complex ones 
> (where you need the refid's). especially the fact that users need to do 
> things significantly different when there are fairly subtle changes to the 
> topology.
> 
> the scenerio that came up elsewhere today where you have
> 
>    Master
>    /    \
> dev1   dev2
> 
> and then dev1 and dev2 both start working on the same thing (without 
> knowing it), then discover they are working on the same thing. they now 
> have threeB options
> 
> 1. merge their stuff up to the master so that they can both pull it down.
>   but this puts broken, experimental stuff up in the master
> 
> 2. declare one of the dev trees to be the master
> 
> this changes the topology to
> 
> Master--dev1--dev2
> 
> 3. pull from each other frequently to keep in sync.
> 
> this changes the topology to
> 
>    Master
>    /   \
> dev1--dev2
> 
> if they do this with bzr then the revno's break, they each get extra 
> commits showing up (so they can never show the same history).

That's a deficiency of merge not telling that a merge is pointless.
Actually I think than bzr merge *should* reduce to pull in all cases:

- If the common ancestor is on the leftmost path of the other branch,
  than the existing revnos as seen on this branch will not change in any
  case, only more than one is added. I think it's safe for merge to
  reduce to pull in this case and consider it a bug in bzr that it does
  not.
- If the common ancestor is not on the leftmost path on the other
  branch, than it is because the branch was merged with some other
  deemed "more important" (ie. the "Master" above). In this case
  reducing to pull will change the old revids, but IMO it's correct
  thing to do, because it's now up-to-date with latest revision of
  "Master" and it's revnos should take precedence. Personally I'd just
  like merge to reduce to pull in this case as well, but maybe it'd be
  better to have it error out and request user to either "pull" or
  "merge --pointless".

> in git this is a non-issue, they can pull back and forth and the only new 
> history to show up will be changes.
> 
> this is the situation that the kernel developers are in frequently. it 
> sounds as if you haven't needed to do this yet, so you haven't encountered 
> the problems.

Given that bzr is considerably smaller project than the Linux kernel,
that's quite likely. And it's likely a reason why it was not thoroughly
discussed in bzr yet (or at least I don't know about that it was).

--------------------------------------------------------------------------------

^ permalink raw reply

* Re: WARNING: THIS PATCH CAN BREAK YOUR REPO, was Re: [PATCH 2/3] Only repack active packs by skipping over kept packs.
From: Junio C Hamano @ 2006-10-30 21:48 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Nicolas Pitre, git
In-Reply-To: <20061030202611.GA5775@spearce.org>

Shawn Pearce <spearce@spearce.org> writes:

> Do the above so you have everything in one pack.  Now use rev-list
> to simulate the object list construction in pack-objects as though
> we were doing a 'git repack -a -d':
>
>   git-rev-list --objects --all \
>     --unpacked=.git/objects/pack/pack-*.pack \
> 	| wc -l
>
> gives me 102 (WRONG WRONG WRONG WRONG!!!!!!)

Now I think I know what is going on.

The meaning of "unpacked" (with or without the "pretend as if
all objects in this pack are loose") has always been to stop
traversing once we hit a packed object, not "do not include
already packed object".

So --unpacked=pretend-this-is-loose was wrong to begin with; it
probably should have been --incremental=pretend-this-is-loose.

How about reverting the following:

commit ce8590748b918687abc4c7cd2d432dd23f07ae40
Author: Shawn Pearce <spearce@spearce.org>

    Only repack active packs by skipping over kept packs.


commit 106d710bc13f34aec1a15c4cff80f062f384edf6
Author: Junio C Hamano <junkio@cox.net>

    pack-objects --unpacked=<existing pack> option.



^ permalink raw reply

* Re: [PATCH/RFC (take 2)] gitweb: New improved patchset view
From: Jakub Narebski @ 2006-10-30 21:50 UTC (permalink / raw)
  To: ltuikov; +Cc: git
In-Reply-To: <850923.44762.qm@web31812.mail.mud.yahoo.com>

Luben Tuikov wrote:
> --- Jakub Narebski <jnareb@gmail.com> wrote:
>>> Wouldn't this be confusing with the other fine lines?
>>> I personally don't like this chunk separation.  Chunk separation
>>> already exists as is and we view it all the time elsewhere.
>> 
>> But not always the program displaying diff can display such line
>> separating chunks, for example on text terminal it can't.
> 
> What I meant is that since I stare at diffs exactly on text terminals,
> my eyes have found other ways to discern chunk blocks.

I'm just saying that with HTML diffs, presented via gitweb in graphical
web browser, you have more possibilities, more formatting to use.
Why not make use of it?

>> But if you think that the dotted 1px #ffbbff line is too intrusive,
>> we can remove it (and perhaps increase vertical space a few pixels).
>> I'd like to have more opinions first.
> 
> No, I just think that it should be as close as possible to what
> we see now and what we see on text terminals -- no extra vertical
> space please.  Between the two evils, I'd prefer the thin "dotted" line.

Well, I'll make it nearly invisible in the "take 3". BTW. some people
liked this line, some were indifferent.

>> BTW. you can easily override it in your CSS file.
> 
> Why should we allow something to go into gitweb and disrupt the current
> default behavior only so that people have to change their own css file
> to keep current default behaviour.  Please don't shove this down our
> throats.  Please?

That was just to note that if you don't agree with default, you can change
it very easily. It is probably the time where people would disagree (for
example infamous "redundant links" debate) on the gitweb UI; the possibility
to tailor it easily to your own UI concepts and ideas is in my opinion
very important (and very nice).

-- 
Jakub Narebski

^ permalink raw reply

* Re: WARNING: THIS PATCH CAN BREAK YOUR REPO, was Re: [PATCH 2/3] Only repack active packs by skipping over kept packs.
From: Shawn Pearce @ 2006-10-30 21:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nicolas Pitre, git
In-Reply-To: <7v7iyhwk47.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> Shawn Pearce <spearce@spearce.org> writes:
> 
> > Do the above so you have everything in one pack.  Now use rev-list
> > to simulate the object list construction in pack-objects as though
> > we were doing a 'git repack -a -d':
> >
> >   git-rev-list --objects --all \
> >     --unpacked=.git/objects/pack/pack-*.pack \
> > 	| wc -l
> >
> > gives me 102 (WRONG WRONG WRONG WRONG!!!!!!)
> 
> Now I think I know what is going on.
> 
> The meaning of "unpacked" (with or without the "pretend as if
> all objects in this pack are loose") has always been to stop
> traversing once we hit a packed object, not "do not include
> already packed object".

Did you see Jan Harkes' patch that changes the behavior to be what
it should have been?
 
> So --unpacked=pretend-this-is-loose was wrong to begin with; it
> probably should have been --incremental=pretend-this-is-loose.

I don't care about what the option name is.  If you want to change
it to --incremental we can but the --unpacked actually makes more
sense now...  Its saying pretend every object in this pack is
unpacked and therefore should be packed.
 
> How about reverting the following:
> 
> commit ce8590748b918687abc4c7cd2d432dd23f07ae40
> Author: Shawn Pearce <spearce@spearce.org>
> 
>     Only repack active packs by skipping over kept packs.
> 
> 
> commit 106d710bc13f34aec1a15c4cff80f062f384edf6
> Author: Junio C Hamano <junkio@cox.net>
> 
>     pack-objects --unpacked=<existing pack> option.
> 

Nah.  I think Jan's patch fixes the bug and the --unpacked option
now makes sense as is, so I don't see why we would revert these.

-- 

^ permalink raw reply

* Re: WARNING: THIS PATCH CAN BREAK YOUR REPO, was Re: [PATCH 2/3] Only repack active packs by skipping over kept packs.
From: Junio C Hamano @ 2006-10-30 21:59 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Nicolas Pitre, git
In-Reply-To: <20061030205200.GA20236@delft.aura.cs.cmu.edu>

Jan Harkes <jaharkes@cs.cmu.edu> writes:

> The following patch does fix the problem Nicolas reported, but for some
> reason I'm still getting only 102 objects (only tags and the commits
> they refer to?) with your test.

One potential downside of this is that this makes an obscure but
useful "gitk --unpacked" useless (robs performance).

http://thread.gmane.org/gmane.comp.version-control.git/19197/focus=19207

But other than that, I think it is an Ok change.  The original
semantics of --unpacked (with or without "pretend as if objects
in this pack are loose") were, eh, "strange".


^ permalink raw reply

* Re: WARNING: THIS PATCH CAN BREAK YOUR REPO, was Re: [PATCH 2/3] Only repack active packs by skipping over kept packs.
From: Junio C Hamano @ 2006-10-30 22:07 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Nicolas Pitre, git
In-Reply-To: <20061030215529.GC5775@spearce.org>

Shawn Pearce <spearce@spearce.org> writes:

> Did you see Jan Harkes' patch that changes the behavior to be what
> it should have been?
>  
>> So --unpacked=pretend-this-is-loose was wrong to begin with; it
>> probably should have been --incremental=pretend-this-is-loose.
>
> I don't care about what the option name is.

It is not about name, but what --unpacked means.  See my other
mail.

^ permalink raw reply

* Re: [PATCH] gitweb: esc_html() author in blame
From: Junio C Hamano @ 2006-10-30 22:16 UTC (permalink / raw)
  To: ltuikov; +Cc: git
In-Reply-To: <20061030203755.5021.qmail@web31810.mail.mud.yahoo.com>

Luben Tuikov <ltuikov@yahoo.com> writes:

> Blame fails for example on
> block/ll_rw_blk.c at v2.6.19-rc3.
>
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>

Some quoting may be needed there, but is esc_html the right quoting?
Ack, somebody?

^ permalink raw reply

* Re: [PATCH/RFC (take 2)] gitweb: New improved patchset view
From: Edgar Toernig @ 2006-10-30 22:30 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: ltuikov, git
In-Reply-To: <200610302250.06733.jnareb@gmail.com>

Jakub Narebski wrote:
>
> I'm just saying that with HTML diffs, presented via gitweb in graphical
> web browser, you have more possibilities, more formatting to use.

It would be nice though, when the gitweb output would be readable
on non css-capable browsers (i.e. w3m) too.  At the moment, gitweb
is mostly usable - the only problematic case is code and diffs.
These are presented via div-tags so in a non-css browser, all spaces
are collapsed thereby removing all indentation.  Couldn't code
fragments be presented via (styled) pre-tags for backward compatibility?
Pretty please :)

Btw, while the css version looks nice, Opera seems to have extreme
performance problems with gitweb's project page when there are a lot
of repositories.  I.e. trying to view http://gitweb.freedesktop.org/
brings my system to its knees.  Turning off style sheets cures it
but then diffs are unusable ...


^ permalink raw reply

* [PATCH] Remove unused variable in receive-pack.
From: Shawn Pearce @ 2006-10-30 22:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

We aren't using this return code variable for anything so lets
just get rid of it to keep this section of code clean.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 receive-pack.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/receive-pack.c b/receive-pack.c
index 7e4e510..f6f1729 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -219,7 +219,7 @@ static void read_head_info(void)
 	}
 }
 
-static const char *unpack(int *error_code)
+static const char *unpack()
 {
 	int code;
 
@@ -230,7 +230,6 @@ static const char *unpack(int *error_cod
 		code = run_command_v_opt(ARRAY_SIZE(unpacker) - 1,
 					 unpacker, RUN_GIT_CMD);
 
-	*error_code = 0;
 	switch (code) {
 	case 0:
 		return NULL;
@@ -247,7 +246,6 @@ static const char *unpack(int *error_cod
 	case -ERR_RUN_COMMAND_WAITPID_NOEXIT:
 		return "unpacker died strangely";
 	default:
-		*error_code = -code;
 		return "unpacker exited with error code";
 	}
 }
@@ -301,8 +299,7 @@ int main(int argc, char **argv)
 
 	read_head_info();
 	if (commands) {
-		int code;
-		const char *unpack_status = unpack(&code);
+		const char *unpack_status = unpack();
 		if (!unpack_status)
 			execute_commands();
 		if (report_status)
-- 
1.4.3.3.g7d63

^ permalink raw reply related

* [PATCH] Move deny_non_fast_forwards handling completely into receive-pack.
From: Shawn Pearce @ 2006-10-30 22:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

The 'receive.denynonfastforwards' option has nothing to do with
the repository format version.  Since receive-pack already uses
git_config to initialize itself before executing any updates we
can use the normal configuration strategy and isolate the receive
specific variables away from the core variables.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 cache.h        |    1 -
 environment.c  |    1 -
 receive-pack.c |   16 +++++++++++++++-
 setup.c        |    2 --
 4 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/cache.h b/cache.h
index a360528..e997a85 100644
--- a/cache.h
+++ b/cache.h
@@ -189,7 +189,6 @@ extern int prefer_symlink_refs;
 extern int log_all_ref_updates;
 extern int warn_ambiguous_refs;
 extern int shared_repository;
-extern int deny_non_fast_forwards;
 extern const char *apply_default_whitespace;
 extern int zlib_compression_level;
 
diff --git a/environment.c b/environment.c
index 63b1d15..84d870c 100644
--- a/environment.c
+++ b/environment.c
@@ -20,7 +20,6 @@ int warn_ambiguous_refs = 1;
 int repository_format_version;
 char git_commit_encoding[MAX_ENCODING_LENGTH] = "utf-8";
 int shared_repository = PERM_UMASK;
-int deny_non_fast_forwards = 0;
 const char *apply_default_whitespace;
 int zlib_compression_level = Z_DEFAULT_COMPRESSION;
 int pager_in_use;
diff --git a/receive-pack.c b/receive-pack.c
index f6f1729..e966148 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -12,12 +12,26 @@ static const char *keep_packer[] = {
 	"index-pack", "--stdin", "--fix-thin", NULL
 };
 
+static int deny_non_fast_forwards = 0;
 static int report_status;
 static int keep_pack;
 
 static char capabilities[] = "report-status keep-pack";
 static int capabilities_sent;
 
+static int receive_pack_config(const char *var, const char *value)
+{
+	git_default_config(var, value);
+
+	if (strcmp(var, "receive.denynonfastforwards") == 0)
+	{
+		deny_non_fast_forwards = git_config_bool(var, value);
+		return 0;
+	}
+
+	return 0;
+}
+
 static int show_ref(const char *path, const unsigned char *sha1, int flag, void *cb_data)
 {
 	if (capabilities_sent)
@@ -290,7 +304,7 @@ int main(int argc, char **argv)
 		die("'%s': unable to chdir or not a git archive", dir);
 
 	setup_ident();
-	git_config(git_default_config);
+	git_config(receive_pack_config);
 
 	write_head_info();
 
diff --git a/setup.c b/setup.c
index 9a46a58..2afdba4 100644
--- a/setup.c
+++ b/setup.c
@@ -244,8 +244,6 @@ int check_repository_format_version(cons
                repository_format_version = git_config_int(var, value);
 	else if (strcmp(var, "core.sharedrepository") == 0)
 		shared_repository = git_config_perm(var, value);
-	else if (strcmp(var, "receive.denynonfastforwards") == 0)
-		deny_non_fast_forwards = git_config_bool(var, value);
        return 0;
 }
 
-- 
1.4.3.3.g7d63

^ permalink raw reply related

* [PATCH] Revert "send-pack --keep: do not explode into loose objects on the receiving end."
From: Shawn Pearce @ 2006-10-30 22:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This reverts commit c7740a943ec896247ebc5514b6be02710caf3c53.

Both Nico and I feel that this is a local matter.   It should not
be decided by the pushing end of the connection.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 receive-pack.c       |   19 +++----------------
 send-pack.c          |   23 +++++------------------
 sha1_file.c          |    7 ++-----
 t/t5400-send-pack.sh |    9 ---------
 4 files changed, 10 insertions(+), 48 deletions(-)

diff --git a/receive-pack.c b/receive-pack.c
index e966148..7e154c5 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -8,15 +8,11 @@
 static const char receive_pack_usage[] = "git-receive-pack <git-dir>";
 
 static const char *unpacker[] = { "unpack-objects", NULL };
-static const char *keep_packer[] = {
-	"index-pack", "--stdin", "--fix-thin", NULL
-};
 
 static int deny_non_fast_forwards = 0;
 static int report_status;
-static int keep_pack;
 
-static char capabilities[] = "report-status keep-pack";
+static char capabilities[] = "report-status";
 static int capabilities_sent;
 
 static int receive_pack_config(const char *var, const char *value)
@@ -219,8 +215,6 @@ static void read_head_info(void)
 		if (reflen + 82 < len) {
 			if (strstr(refname + reflen + 1, "report-status"))
 				report_status = 1;
-			if (strstr(refname + reflen + 1, "keep-pack"))
-				keep_pack = 1;
 		}
 		cmd = xmalloc(sizeof(struct command) + len - 80);
 		hashcpy(cmd->old_sha1, old_sha1);
@@ -235,14 +229,7 @@ static void read_head_info(void)
 
 static const char *unpack()
 {
-	int code;
-
-	if (keep_pack)
-		code = run_command_v_opt(ARRAY_SIZE(keep_packer) - 1,
-					 keep_packer, RUN_GIT_CMD);
-	else
-		code = run_command_v_opt(ARRAY_SIZE(unpacker) - 1,
-					 unpacker, RUN_GIT_CMD);
+	int code = run_command_v_opt(1, unpacker, RUN_GIT_CMD);
 
 	switch (code) {
 	case 0:
@@ -300,7 +287,7 @@ int main(int argc, char **argv)
 	if (!dir)
 		usage(receive_pack_usage);
 
-	if (!enter_repo(dir, 0))
+	if(!enter_repo(dir, 0))
 		die("'%s': unable to chdir or not a git archive", dir);
 
 	setup_ident();
diff --git a/send-pack.c b/send-pack.c
index 0e90548..fbd792c 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -6,14 +6,13 @@
 #include "exec_cmd.h"
 
 static const char send_pack_usage[] =
-"git-send-pack [--all] [--keep] [--exec=git-receive-pack] <remote> [<head>...]\n"
+"git-send-pack [--all] [--exec=git-receive-pack] <remote> [<head>...]\n"
 "  --all and explicit <head> specification are mutually exclusive.";
 static const char *exec = "git-receive-pack";
 static int verbose;
 static int send_all;
 static int force_update;
 static int use_thin_pack;
-static int keep_pack;
 
 static int is_zero_sha1(const unsigned char *sha1)
 {
@@ -271,7 +270,6 @@ static int send_pack(int in, int out, in
 	int new_refs;
 	int ret = 0;
 	int ask_for_status_report = 0;
-	int ask_to_keep_pack = 0;
 	int expect_status_report = 0;
 
 	/* No funny business with the matcher */
@@ -281,8 +279,6 @@ static int send_pack(int in, int out, in
 	/* Does the other end support the reporting? */
 	if (server_supports("report-status"))
 		ask_for_status_report = 1;
-	if (server_supports("keep-pack") && keep_pack)
-		ask_to_keep_pack = 1;
 
 	/* match them up */
 	if (!remote_tail)
@@ -359,17 +355,12 @@ static int send_pack(int in, int out, in
 		strcpy(old_hex, sha1_to_hex(ref->old_sha1));
 		new_hex = sha1_to_hex(ref->new_sha1);
 
-		if (ask_for_status_report || ask_to_keep_pack) {
-			packet_write(out, "%s %s %s%c%s%s",
+		if (ask_for_status_report) {
+			packet_write(out, "%s %s %s%c%s",
 				     old_hex, new_hex, ref->name, 0,
-				     ask_for_status_report
-				     ? " report-status" : "",
-				     ask_to_keep_pack
-				     ? " keep-pack" : "");
-			if (ask_for_status_report)
-				expect_status_report = 1;
+				     "report-status");
 			ask_for_status_report = 0;
-			ask_to_keep_pack = 0;
+			expect_status_report = 1;
 		}
 		else
 			packet_write(out, "%s %s %s",
@@ -428,10 +419,6 @@ int main(int argc, char **argv)
 				verbose = 1;
 				continue;
 			}
-			if (!strcmp(arg, "--keep")) {
-				keep_pack = 1;
-				continue;
-			}
 			if (!strcmp(arg, "--thin")) {
 				use_thin_pack = 1;
 				continue;
diff --git a/sha1_file.c b/sha1_file.c
index 59883a9..5e6c8b8 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1310,7 +1310,7 @@ static void *read_packed_sha1(const unsi
 	return unpack_entry(&e, type, size);
 }
 
-void *read_sha1_file(const unsigned char *sha1, char *type, unsigned long *size)
+void * read_sha1_file(const unsigned char *sha1, char *type, unsigned long *size)
 {
 	unsigned long mapsize;
 	void *map, *buf;
@@ -1775,10 +1775,7 @@ int has_sha1_file(const unsigned char *s
 
 	if (find_pack_entry(sha1, &e, NULL))
 		return 1;
-	if (find_sha1_file(sha1, &st))
-		return 1;
-	reprepare_packed_git();
-	return find_pack_entry(sha1, &e, NULL);
+	return find_sha1_file(sha1, &st) ? 1 : 0;
 }
 
 /*
diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index d831f8d..8afb899 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -78,13 +78,4 @@ test_expect_success \
 	! diff -u .git/refs/heads/master victim/.git/refs/heads/master
 '
 
-test_expect_success 'push with --keep' '
-	t=`cd victim && git-rev-parse --verify refs/heads/master` &&
-	git-update-ref refs/heads/master $t &&
-	: > foo &&
-	git add foo &&
-	git commit -m "one more" &&
-	git-send-pack --keep ./victim/.git/ master
-'
-
 test_done
-- 
1.4.3.3.g7d63

^ permalink raw reply related

* [PATCH] Teach receive-pack how to keep pack files when unpacklooseobjects = 0.
From: Shawn Pearce @ 2006-10-30 22:36 UTC (permalink / raw)
  To: Junio C Hamano; +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.unpackLooseObjects.

If receive.unpackLooseObjects is true (which it is by default for
now) then we unpack-objects as we have in the past.

If it is false 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 |    9 ++++++++-
 cache.h                  |    1 +
 receive-pack.c           |   35 ++++++++++++++++++++++++++++++++---
 sha1_file.c              |    2 +-
 4 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index d9e73da..4eab9e8 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -301,7 +301,14 @@ imap::
 	The configuration variables in the 'imap' section are described
 	in gitlink:git-imap-send[1].
 
-receive.denyNonFastforwads::
+receive.unpackLooseObjects::
+	If set to true, git-receive-pack will unpack each received
+	object into a loose object in the repository.  If set to
+	false then the received pack file will be kept as is (but
+	may have delta bases appended onto the end).  Large pushes
+	into a repository will generally go faster if this is false.
+
+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..6e377f0 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -7,9 +7,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_loose_objects = 1;
 static int report_status;
 
 static char capabilities[] = "report-status";
@@ -25,6 +24,12 @@ static int receive_pack_config(const cha
 		return 0;
 	}
 
+	if (strcmp(var, "receive.unpacklooseobjects") == 0)
+	{
+		unpack_loose_objects = git_config_bool(var, value);
+		return 0;
+	}
+
 	return 0;
 }
 
@@ -229,7 +234,31 @@ static void read_head_info(void)
 
 static const char *unpack()
 {
-	int code = run_command_v_opt(1, unpacker, RUN_GIT_CMD);
+	static const char *unpacker[] = { "unpack-objects", NULL };
+	int code;
+	
+	if (unpack_loose_objects)
+		code = run_command_v_opt(1, unpacker, RUN_GIT_CMD);
+	else {
+		const char *keeper[5], *why_kept;
+		char my_host[255], keep_arg[128 + 255];
+
+		if (gethostname(my_host, sizeof(my_host)))
+			strcpy(my_host, "localhost");
+		snprintf(keep_arg, sizeof(keep_why),
+			"--keep=receive-pack %i on %s",
+			getpid(), my_host);
+		why_kept = keep_arg + 7;
+
+		keeper[0] = "index-pack";
+		keeper[1] = "--stdin";
+		keeper[2] = "--fix-thin";
+		keeper[3] = keep_arg;
+		keeper[4] = 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

* Re: [PATCH/RFC (take 2)] gitweb: New improved patchset view
From: Jakub Narebski @ 2006-10-30 22:39 UTC (permalink / raw)
  To: Edgar Toernig; +Cc: ltuikov, git
In-Reply-To: <20061030233017.19f25117.froese@gmx.de>

Edgar Toernig wrote:
> Jakub Narebski wrote:
>>
>> I'm just saying that with HTML diffs, presented via gitweb in graphical
>> web browser, you have more possibilities, more formatting to use.
> 
> It would be nice though, when the gitweb output would be readable
> on non css-capable browsers (i.e. w3m) too.  At the moment, gitweb
> is mostly usable - the only problematic case is code and diffs.
> These are presented via div-tags so in a non-css browser, all spaces
> are collapsed thereby removing all indentation.  Couldn't code
> fragments be presented via (styled) pre-tags for backward compatibility?
> Pretty please :)

Well, we replaced using s/ /&nbsp;/g with .pre class woth white-space: pre.
Perhaps we can go halfway, and add <pre>...</pre> wrapping line.
 
> Btw, while the css version looks nice, Opera seems to have extreme
> performance problems with gitweb's project page when there are a lot
> of repositories.  I.e. trying to view http://gitweb.freedesktop.org/
> brings my system to its knees.  Turning off style sheets cures it
> but then diffs are unusable ...

Strange. It's just a simple table. Could you and would you be able to
debug it further (e.g. by bisecting CSS)?

-- 
Jakub Narebski

^ permalink raw reply

* Re: [PATCH/RFC (take 2)] gitweb: New improved patchset view
From: Luben Tuikov @ 2006-10-30 22:40 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200610302250.06733.jnareb@gmail.com>

--- Jakub Narebski <jnareb@gmail.com> wrote:
> I'm just saying that with HTML diffs, presented via gitweb in graphical
> web browser, you have more possibilities, more formatting to use.
> Why not make use of it?

That sounds fine.

The question is where one draws the line.

> >> BTW. you can easily override it in your CSS file.
> > 
> > Why should we allow something to go into gitweb and disrupt the current
> > default behavior only so that people have to change their own css file
> > to keep current default behaviour.  Please don't shove this down our
> > throats.  Please?
> 
> That was just to note that if you don't agree with default, you can change
> it very easily. It is probably the time where people would disagree (for
> example infamous "redundant links" debate) on the gitweb UI; the possibility
> to tailor it easily to your own UI concepts and ideas is in my opinion
> very important (and very nice).

I would like to keep the visual default as stable as possible.

   Luben

^ permalink raw reply

* Re: [PATCH 1/2] Allow '-' in config variable names
From: Junio C Hamano @ 2006-10-30 22:45 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0610300823250.25218@g5.osdl.org>

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.

I do not have problem with this (and would perhaps also want to
add '_' to keychar set), but people who envisioned parsing
config from scripts (i.e. Perly git) might prefer if we stayed
within alnum, since I'd suspect then they may be able to reuse
existing .ini parsers.  I do not much care about that myself,
but I am bringing it up just in case other people might.

Other than that, this sounds nice.

By the way, everybody seems to do "alias.xxx = -p cat-file -p"
(I have it as "git less").  Maybe we would want to make a
built-in alias for that?

^ permalink raw reply

* Re: [PATCH] gitweb: esc_html() author in blame
From: Luben Tuikov @ 2006-10-30 22:49 UTC (permalink / raw)
  To: Junio C Hamano, git
In-Reply-To: <7vu01lv48z.fsf@assigned-by-dhcp.cox.net>

--- Junio C Hamano <junkio@cox.net> wrote:
> Luben Tuikov <ltuikov@yahoo.com> writes:
> 
> > Blame fails for example on
> > block/ll_rw_blk.c at v2.6.19-rc3.
> >
> > Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> 
> Some quoting may be needed there, but is esc_html the right quoting?
> Ack, somebody?

I don't know if it is the right quoting.  esc_html() seemed to be
used elsewhere for $author, and it seemed to work in this particular
case.

    Luben

^ permalink raw reply

* Re: WARNING: THIS PATCH CAN BREAK YOUR REPO, was Re: [PATCH 2/3] Only repack active packs by skipping over kept packs.
From: Junio C Hamano @ 2006-10-30 22:54 UTC (permalink / raw)
  To: Jan Harkes; +Cc: git, Nicolas Pitre, Shawn Pearce
In-Reply-To: <7v3b95wjmg.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> Jan Harkes <jaharkes@cs.cmu.edu> writes:
>
>> The following patch does fix the problem Nicolas reported, but for some
>> reason I'm still getting only 102 objects (only tags and the commits
>> they refer to?) with your test.
>
> One potential downside of this is that this makes an obscure but
> useful "gitk --unpacked" useless (robs performance).
>
> http://thread.gmane.org/gmane.comp.version-control.git/19197/focus=19207
>
> But other than that, I think it is an Ok change.  The original
> semantics of --unpacked (with or without "pretend as if objects
> in this pack are loose") were, eh, "strange".

I changed my mind.

Even without --unpacked=pretend-this-is-loose nor .keep flag,
the original semantics of --unpacked and git repack do not play
with each other well.  You can have a history where you have a
pack in the middle of the history, and would expect "git repack"
without -a to make your .git/objects/??/ directories empty but
it would not because --unpacked has been defined to mean "stop
traversal when we hit a packed commit".  That would _not_
corrupt the repository, but is very counter-intuitive.

Unfortunately this is a semantic change in the middle of the
road (and it would change the _output_ not just performance of
"gitk --unpacked"), but I think it is a semantic change of a
good kind.

So I'll take Jan's patch as is.  It needs to go all the way down
to "maint", since we have --unpacked= there already.

^ permalink raw reply

* Re: WARNING: THIS PATCH CAN BREAK YOUR REPO, was Re: [PATCH 2/3] Only repack active packs by skipping over kept packs.
From: Jan Harkes @ 2006-10-30 22:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v3b95wjmg.fsf@assigned-by-dhcp.cox.net>

On Mon, Oct 30, 2006 at 01:59:03PM -0800, Junio C Hamano wrote:
> Jan Harkes <jaharkes@cs.cmu.edu> writes:
> 
> > The following patch does fix the problem Nicolas reported, but for some
> > reason I'm still getting only 102 objects (only tags and the commits
> > they refer to?) with your test.
> 
> One potential downside of this is that this makes an obscure but
> useful "gitk --unpacked" useless (robs performance).
> 
> http://thread.gmane.org/gmane.comp.version-control.git/19197/focus=19207

If I use 'git fetch' followed later on by a 'git fetch -k', the result
from --unpacked would not include the unpacked objects created by the
first fetch. Although it may have been fast, it seems to be somewhat
counter-intuitive.

> But other than that, I think it is an Ok change.  The original
> semantics of --unpacked (with or without "pretend as if objects
> in this pack are loose") were, eh, "strange".

Do you need a resend with a proper 'Signed-Off-By' line?

Jan

^ 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