Git development
 help / color / mirror / Atom feed
* [PATCH/RFC 1/x] gitweb: Use git-diff-tree patch output for commitdiff
From: Jakub Narebski @ 2006-08-23 22:15 UTC (permalink / raw)
  To: git

As the subject indicates (RFC), I'd like some comments on that patch.


This patch is the first patch in series which tries to remove external
diff dependency in gitweb, and the need for temporary files.

Converting "blobdiff" and "blobdiff_plain" format would be much easier
if git-diff-tree and friends had -L <label>/--label=<label> option,
like GNU diff has.

Current patch preserves much of current output; the question is if for
example generate if 'plain' format should generate patch which could
be appplied by ordinary patch which do not understand git diff
extensions (including renaming and copying), as it is done in current
version, and if 'html' version should detect renames and copying.

Another question is if to have (as it is currently done) 'html' and
'plain' format generated by one subroutine, even though they don't
share that much code, and how to divide this code (currently invoking
git-commit-diff is in one part, generating header and commit message
is in second part, generating patch/patchset is in third part).


Further patches are planned, including getting rid of
git_commitdiff_plain, changing format_diff_line to include incomplete
lines in output, and converting git_blobdiff and git_blobdiff_plain to
use git-diff-tree.

This patch is based on 'next' (fbf19dd41bb51d5221fac739c5bdb48fd9012412)


P.S. Perhaps the below separator should be made standard and
recognized by git tools?
 
-- >8 --
Get rid of git_diff_print invocation in git_commitdiff and therefore
external diff (/usr/bin/diff) invocation, and use only git-diff-tree
to generate patch.

git_commitdiff and git_commitdiff_plain are collapsed into one
subroutine git_commitdiff, with format (currently 'html' which is
default format corresponding to git_commitdiff, and 'plain'
corresponding to git_commitdiff_plain) specified in argument.

Separate patch (diff) pretty-printing into git_patchset_body.
It is used in git_commitdiff.

Separate patch (diff) line formatting from git_diff_print into
format_diff_line function. It is used in git_patchset_body.

While at it, add $hash parameter to git_difftree_body, according to
rule that inner functions should use parameter passing, and not global
variables.

CHANGES TO OUTPUT:
 * "commitdiff" now products patches with renaming and copying
   detection (git-diff-tree is invoked with -M and -C options).
   Empty patches (mode changes and pure renames and copying)
   are not written currently. Former version broke renaming and
   copying, and didn't notice mode changes, like this version.

 * "commitdiff" output is now divided into several div elements
   of class "log", "patchset" and "patch".

 * "commitdiff_plain" now only generates X-Git-Tag: line only if there
   is tag pointing to the current commit. Former version which wrote
   first tag following current commit was broken[*1*]; besides we are
   interested rather in tags _preceding_ the commit, and _heads_
   following the commit. X-Git-Url: now is current URL; former version
   tried[*2*] to output URL to HTML version of commitdiff.

 * "commitdiff_plain" is generated by git-diff-tree, and has therefore
   has git specific extensions to diff format: "git diff" header and
   optional extended header lines.

FOOTNOTES
[*1*] First it generated rev-list starting from HEAD even if hash_base
parameter was set, second it wasn't corrected according to changes
made in git_get_references (formerly read_info_ref) output, third even
for older version of read_info_ref output it didn't work for multiple
tags pointing to the current commit (rare).

[*2*] It wrote URL for commitdiff without hash_parent, which produces
diff to first parent and is not the same as current diff if it is diff
of merge commit to non-first parent.

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 50083e3..9367685 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -524,6 +524,27 @@ sub format_subject_html {
 	}
 }
 
+sub format_diff_line {
+	my $line = shift;
+	my $char = substr($line, 0, 1);
+	my $diff_class = "";
+
+	chomp $line;
+
+	if ($char eq '+') {
+		$diff_class = " add";
+	} elsif ($char eq "-") {
+		$diff_class = " rem";
+	} elsif ($char eq "@") {
+		$diff_class = " chunk_header";
+	} elsif ($char eq "\\") {
+		# skip errors (incomplete lines)
+		return "";
+	}
+	$line = untabify($line);
+	return "<div class=\"diff$diff_class\">" . esc_html($line) . "</div>\n";
+}
+
 ## ----------------------------------------------------------------------
 ## git utility subroutines, invoking git commands
 
@@ -1367,7 +1388,7 @@ ## .....................................
 ## functions printing large fragments of HTML
 
 sub git_difftree_body {
-	my ($difftree, $parent) = @_;
+	my ($difftree, $hash, $parent) = @_;
 
 	print "<div class=\"list_head\">\n";
 	if ($#{$difftree} > 10) {
@@ -1518,6 +1539,101 @@ sub git_difftree_body {
 	print "</table>\n";
 }
 
+sub git_patchset_body {
+	my ($patchset, $difftree, $hash, $hash_parent) = @_;
+
+	my $patch_idx = 0;
+	my $in_header = 0;
+	my $patch_found = 0;
+	my %diffinfo;
+
+	print "<div class=\"patchset\">\n";
+
+	LINE: foreach my $patch_line (@$patchset) {
+
+		if ($patch_line =~ m/^diff /) { # "git diff" header
+			# beginning of patch (in patchset)
+			if ($patch_found) {
+				# close previous patch
+				print "</div>\n"; # class="patch"
+			} else {
+				# first patch in patchset
+				$patch_found = 1;
+			}
+			print "<div class=\"patch\">\n";
+
+			%diffinfo = parse_difftree_raw_line($difftree->[$patch_idx++]);
+
+			# for now, no extended header, hence we skip empty patches
+			# companion to	next LINE if $in_header;
+			if ($diffinfo{'from_id'} eq $diffinfo{'to_id'}) { # no change
+				$in_header = 1;
+				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
+				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 "<div class=\"diff extended_header\">\n";
+			$in_header = 1;
+			next LINE;
+		} # start of patch in patchset
+
+
+		if ($in_header && $patch_line =~ m/^---/) {
+			#print "</div>\n"
+			$in_header = 0;
+		}
+		next LINE if $in_header;
+
+		print format_diff_line($patch_line);
+	}
+	print "</div>\n" if $patch_found; # class="patch"
+
+	print "</div>\n"; # class="patchset"
+}
+
+# . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
+
 sub git_shortlog_body {
 	# uses global variable $project
 	my ($revlist, $from, $to, $refs, $extra) = @_;
@@ -2556,7 +2672,7 @@ sub git_commit {
 	git_print_log($co{'comment'});
 	print "</div>\n";
 
-	git_difftree_body(\@difftree, $parent);
+	git_difftree_body(\@difftree, $hash, $parent);
 
 	git_footer_html();
 }
@@ -2600,7 +2716,7 @@ sub git_blobdiff_plain {
 }
 
 sub git_commitdiff {
-	mkdir($git_temp, 0700);
+	my $format = shift || 'html';
 	my %co = parse_commit($hash);
 	if (!%co) {
 		die_error(undef, "Unknown commit object");
@@ -2608,143 +2724,106 @@ sub git_commitdiff {
 	if (!defined $hash_parent) {
 		$hash_parent = $co{'parent'} || '--root';
 	}
-	open my $fd, "-|", $GIT, "diff-tree", '-r', $hash_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");
+
+	# read commitdiff
+	my $fd;
+	my @difftree;
+	my @patchset;
+	if ($format eq 'html') {
+		open $fd, "-|", $GIT, "diff-tree", '-r', '-M', '-C',
+			"--patch-with-raw", "--full-index", $hash_parent, $hash
+			or die_error(undef, "Open git-diff-tree failed");
+
+		while (chomp(my $line = <$fd>)) {
+			# empty line ends raw part of diff-tree output
+			last unless $line;
+			push @difftree, $line;
+		}
+		@patchset = map { chomp; $_ } <$fd>;
+
+		close $fd
+			or die_error(undef, "Reading git-diff-tree failed");
+	} elsif ($format eq 'plain') {
+		open $fd, "-|", $GIT, "diff-tree", '-r', '-p', '-B', $hash_parent, $hash
+			or die_error(undef, "Open git-diff-tree failed");
+	} else {
+		die_error(undef, "Unknown commitdiff format");
+	}
 
 	# non-textual hash id's can be cached
 	my $expires;
 	if ($hash =~ m/^[0-9a-fA-F]{40}$/) {
 		$expires = "+1d";
 	}
-	my $refs = git_get_references();
-	my $ref = format_ref_marker($refs, $co{'id'});
-	my $formats_nav =
-		$cgi->a({-href => href(action=>"commitdiff_plain", hash=>$hash, hash_parent=>$hash_parent)},
-		        "plain");
-	git_header_html(undef, $expires);
-	git_print_page_nav('commitdiff','', $hash,$co{'tree'},$hash, $formats_nav);
-	git_print_header_div('commit', esc_html($co{'title'}) . $ref, $hash);
-	print "<div class=\"page_body\">\n";
-	git_print_simplified_log($co{'comment'}, 1); # skip title
-	print "<br/>\n";
-	foreach my $line (@difftree) {
-		# ':100644 100644 03b218260e99b78c6df0ed378e59ed9205ccc96d 3b93d5e7cc7f7dd4ebed13a5cc1a4ad976fc94d8 M      ls-files.c'
-		# ':100644 100644 7f9281985086971d3877aca27704f2aaf9c448ce bc190ebc71bbd923f2b728e505408f5e54bd073a M      rev-tree.c'
-		if ($line !~ m/^:([0-7]{6}) ([0-7]{6}) ([0-9a-fA-F]{40}) ([0-9a-fA-F]{40}) (.)\t(.*)$/) {
-			next;
-		}
-		my $from_mode = $1;
-		my $to_mode = $2;
-		my $from_id = $3;
-		my $to_id = $4;
-		my $status = $5;
-		my $file = validate_input(unquote($6));
-		if ($status eq "A") {
-			print "<div class=\"diff_info\">" . file_type($to_mode) . ":" .
-			      $cgi->a({-href => href(action=>"blob", hash_base=>$hash,
-			                             hash=>$to_id, file_name=>$file)},
-			              $to_id) . "(new)" .
-			      "</div>\n";
-			git_diff_print(undef, "/dev/null", $to_id, "b/$file");
-		} elsif ($status eq "D") {
-			print "<div class=\"diff_info\">" . file_type($from_mode) . ":" .
-			      $cgi->a({-href => href(action=>"blob", hash_base=>$hash_parent,
-			                             hash=>$from_id, file_name=>$file)},
-			              $from_id) . "(deleted)" .
-			      "</div>\n";
-			git_diff_print($from_id, "a/$file", undef, "/dev/null");
-		} elsif ($status eq "M") {
-			if ($from_id ne $to_id) {
-				print "<div class=\"diff_info\">" .
-				      file_type($from_mode) . ":" .
-				      $cgi->a({-href => href(action=>"blob", hash_base=>$hash_parent,
-				                             hash=>$from_id, file_name=>$file)},
-				              $from_id) .
-				      " -> " .
-				      file_type($to_mode) . ":" .
-				      $cgi->a({-href => href(action=>"blob", hash_base=>$hash,
-				                             hash=>$to_id, file_name=>$file)},
-				              $to_id);
-				print "</div>\n";
-				git_diff_print($from_id, "a/$file",  $to_id, "b/$file");
-			}
-		}
-	}
-	print "<br/>\n" .
-	      "</div>";
-	git_footer_html();
-}
 
-sub git_commitdiff_plain {
-	mkdir($git_temp, 0700);
-	my %co = parse_commit($hash);
-	if (!%co) {
-		die_error(undef, "Unknown commit object");
-	}
-	if (!defined $hash_parent) {
-		$hash_parent = $co{'parent'} || '--root';
-	}
-	open my $fd, "-|", $GIT, "diff-tree", '-r', $hash_parent, $hash
-		or die_error(undef, "Open git-diff-tree failed");
-	my @difftree = map { chomp; $_ } <$fd>;
-	close $fd or die_error(undef, "Reading diff-tree failed");
-
-	# try to figure out the next tag after this commit
-	my $tagname;
-	my $refs = git_get_references("tags");
-	open $fd, "-|", $GIT, "rev-list", "HEAD";
-	my @commits = map { chomp; $_ } <$fd>;
-	close $fd;
-	foreach my $commit (@commits) {
-		if (defined $refs->{$commit}) {
-			$tagname = $refs->{$commit}
-		}
-		if ($commit eq $hash) {
-			last;
-		}
-	}
+	# write commit message
+	if ($format eq 'html') {
+		my $refs = git_get_references();
+		my $ref = format_ref_marker($refs, $co{'id'});
+		my $formats_nav =
+			$cgi->a({-href => href(action=>"commitdiff_plain",
+			                       hash=>$hash, hash_parent=>$hash_parent)},
+			        "plain");
 
-	print $cgi->header(-type => "text/plain",
-	                   -charset => 'utf-8',
-	                   -content_disposition => "inline; filename=\"git-$hash.patch\"");
-	my %ad = parse_date($co{'author_epoch'}, $co{'author_tz'});
-	my $comment = $co{'comment'};
-	print <<TEXT;
+		git_header_html(undef, $expires);
+		git_print_page_nav('commitdiff','', $hash,$co{'tree'},$hash, $formats_nav);
+		git_print_header_div('commit', esc_html($co{'title'}) . $ref, $hash);
+		print "<div class=\"page_body\">\n";
+		print "<div class=\"log\">\n";
+		git_print_simplified_log($co{'comment'}, 1); # skip title
+		print "</div>\n"; # class="log"
+
+	} elsif ($format eq 'plain') {
+		my $refs = git_get_references("tags");
+		my @tagnames;
+		if (exists $refs->{$hash}) {
+			@tagnames = map { s|^tags/|| } $refs->{$hash};
+		}
+		my $filename = basename($project) . "-$hash.patch";
+
+		print $cgi->header(
+			-type => 'text/plain',
+			-charset => 'utf-8',
+			-expires => $expires,
+			-content_disposition => qq(inline; filename="$filename"));
+		my %ad = parse_date($co{'author_epoch'}, $co{'author_tz'});
+		print <<TEXT;
 From: $co{'author'}
 Date: $ad{'rfc2822'} ($ad{'tz_local'})
 Subject: $co{'title'}
 TEXT
-	if (defined $tagname) {
-		print "X-Git-Tag: $tagname\n";
+		foreach my $tag (@tagnames) {
+			print "X-Git-Tag: $tag\n";
+		}
+		print "X-Git-Url: " . $cgi->self_url() . "\n\n";
+		foreach my $line (@{$co{'comment'}}) {
+			print "$line\n";
+		}
+		print "---\n\n";
 	}
-	print "X-Git-Url: $my_url?p=$project;a=commitdiff;h=$hash\n" .
-	      "\n";
 
-	foreach my $line (@$comment) {;
-		print "$line\n";
-	}
-	print "---\n\n";
+	# write patch
+	if ($format eq 'html') {
+		#git_difftree_body(\@difftree, $hash, $hash_parent);
+		#print "<br/>\n";
 
-	foreach my $line (@difftree) {
-		if ($line !~ m/^:([0-7]{6}) ([0-7]{6}) ([0-9a-fA-F]{40}) ([0-9a-fA-F]{40}) (.)\t(.*)$/) {
-			next;
-		}
-		my $from_id = $3;
-		my $to_id = $4;
-		my $status = $5;
-		my $file = $6;
-		if ($status eq "A") {
-			git_diff_print(undef, "/dev/null", $to_id, "b/$file", "plain");
-		} elsif ($status eq "D") {
-			git_diff_print($from_id, "a/$file", undef, "/dev/null", "plain");
-		} elsif ($status eq "M") {
-			git_diff_print($from_id, "a/$file",  $to_id, "b/$file", "plain");
-		}
+		git_patchset_body(\@patchset, \@difftree, $hash, $hash_parent);
+
+		print "</div>\n"; # class="page_body"
+		git_footer_html();
+
+	} elsif ($format eq 'plain') {
+		local $/ = undef;
+		print <$fd>;
+		close $fd
+			or print "Reading git-diff-tree failed\n";
 	}
 }
 
+sub git_commitdiff_plain {
+	git_commitdiff('plain');
+}
+
 sub git_history {
 	if (!defined $hash_base) {
 		$hash_base = git_get_head_hash($project);
-- 
1.4.1.1

^ permalink raw reply related

* Re: [PATCH] git-daemon virtual hosting implementation.
From: Pierre Habouzit @ 2006-08-23 20:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmz9vgqlm.fsf@assigned-by-dhcp.cox.net>

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

Le mer 23 août 2006 22:11, Junio C Hamano a écrit :
> Pierre Habouzit <madcoder@debian.org> writes:
> > just add the hostname in the path when using --base-path and
> > --user-path. this should be enough for most needs.
> >
> > Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> > ---
> >  Here is a proposal for daemon side virtualhosting support.
> >
> > @@ -158,6 +160,11 @@ static char *path_ok(char *dir)
> >  		return NULL;
> >  	}
> >
> > +	if (use_vhosts && !vhost) {
> > +		logerror("using virtual hosting, and not host= was specified
> > !"); +		return NULL;
> > +	}
> > +
>
> This part is objectionable -- older clients do not give "host=".
> I think the plan, when virtual hosting was proposed and we added
> this to the client side first, was to treat older clients as if
> they specified the "primary" host.  So we would need some
> mechanism to say where the repositories of the "primary" host
> lives.

fair enough, so I suppose there is many choices:
 * replace --use-vhosts with --use-vhosts=${default hostname}
   but I do not like it very much
 * use `hostname -f` as the default hostname (not very nice either)
 * use the magic _default_ (à la apache) hostname ?

But just note that if a git repository is accessible on an host that is 
not the default, and only on that one, an older client won't be able to 
clone the repository anyway, because he will be redirected to the 
default virtual host. Meaning, that someone that uses virtual hosts 
will break older clients anyway.

what would be nicer would be a way to gracefully reject older clients in 
that case with an understandable error message, so that the user knows 
why it does not work. I don't know PROTO_GIT very well yet,so I don't 
know if older client support such communications yet or not.

> > +			if (use_vhosts) {
> > +				loginfo("host <%s>, "
> > +					"userpath <%s>, request <%s>, "
> > +					"namlen %d, restlen %d, slash <%s>",
> > +					vhost,
> > +					user_path, dir,
> > +					namlen, restlen, slash);
> > +				snprintf(rpath, PATH_MAX, "%.*s/%s/%s%.*s",
> > +					 namlen, dir, user_path, vhost,
> > +					 restlen, slash);
>
> I am not sure if the interaction between user-path and vhost
> should go like this, but I do not think of a good alternative to
> suggest right now.  Your code allows ~user/host1 and ~user/host2
> to host different set of repositories, but I suspect if somebody
> is setting up a virtual hosting of two hosts, he might want to
> have two distinct set of users (iow to pretend that ~user exist
> only on host1 but not on host2).  I dunno.

Another option would be not to support virtual hosts, but instead 
superseed the --base-path and --user-path with some --base-path-fmt 
and --user-path-fmt where the user can specify how to build the path 
with simple sprintf-like formats. One could e.g. support:
 * %% obviously ;
 * %h that will be replaced with the hostname
 * %u (only for --user-path-fmt)
 * %p (asked path)
 * ...

I think that's more clever, and allow more flexible use of the virtual 
hosting code. It e.g. allow to use the virtual host scheme for the 
`base-path` repos and to disallow it for the users.

--*-path and --*-path-fmt are obviously mutually exclusive.

What do you think ?

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ permalink raw reply

* Re: [PATCH] git-daemon virtual hosting implementation.
From: Junio C Hamano @ 2006-08-23 20:11 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git
In-Reply-To: <11563591572194-git-send-email-madcoder@debian.org>

Pierre Habouzit <madcoder@debian.org> writes:

> just add the hostname in the path when using --base-path and --user-path.
> this should be enough for most needs.
>
> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
>  Here is a proposal for daemon side virtualhosting support.

> @@ -158,6 +160,11 @@ static char *path_ok(char *dir)
>  		return NULL;
>  	}
>  
> +	if (use_vhosts && !vhost) {
> +		logerror("using virtual hosting, and not host= was specified !");
> +		return NULL;
> +	}
> +

This part is objectionable -- older clients do not give "host=".
I think the plan, when virtual hosting was proposed and we added
this to the client side first, was to treat older clients as if
they specified the "primary" host.  So we would need some
mechanism to say where the repositories of the "primary" host
lives.

> +			if (use_vhosts) {
> +				loginfo("host <%s>, "
> +					"userpath <%s>, request <%s>, "
> +					"namlen %d, restlen %d, slash <%s>",
> +					vhost,
> +					user_path, dir,
> +					namlen, restlen, slash);
> +				snprintf(rpath, PATH_MAX, "%.*s/%s/%s%.*s",
> +					 namlen, dir, user_path, vhost,
> +					 restlen, slash);

I am not sure if the interaction between user-path and vhost
should go like this, but I do not think of a good alternative to
suggest right now.  Your code allows ~user/host1 and ~user/host2
to host different set of repositories, but I suspect if somebody
is setting up a virtual hosting of two hosts, he might want to
have two distinct set of users (iow to pretend that ~user exist
only on host1 but not on host2).  I dunno.

^ permalink raw reply

* Re: Proposal for new git Merge Strategy
From: Junio C Hamano @ 2006-08-23 20:00 UTC (permalink / raw)
  To: Jon Loeliger; +Cc: git
In-Reply-To: <E1GFxeZ-0000Nw-ED@jdl.com>

Jon Loeliger <jdl@jdl.com> writes:

> But for complex or critical merges, a "guided merge"
> strategy seems like it might be a useful tool.  Basically,
> it would offer options to select Stage 1 or Stage 2
> revisions, or step in and offer hunks from Stage 1 and 2,
> revert to "ours" or "theirs", or "revert to 'ours' or 'theirs'
> for all remaining files".  Things like that maybe.
>
> Any thoughts down this line?  Good idea?  Bad idea?

We had some discussion on this with Catalin in "Unresolved
issues #3" thread, regarding git-xxdiff (did I ever take it?  I
liked it for what it does, but I was not sure about its
odd-man-out-ness) which was proposed by Martin Langhoff.

A merge that results in manual fixups conceptually take these
steps:

 - revs involved in 3-way merge identified with git-merge-base;

 - read-tree is given these three bases;

 - git-merge-index gives three stages as individual temporary
   files to git-merge-one-file for each path that cannot be
   resolved at tree-level.

   - git-merge-one-file calls "merge" (reminds me that I should
     replace this with "diff3").

We should be able to make the part that call "merge/diff3" to
alternatively call xxdiff or its friends (kompare, emerge, pick
your favorites).  Catalin even showed us a code snippet used in
StGIT for this in the thread.

Martin's proposed tool git-xxdiff is meant to be invoked after
all of the above still left conflict markers.  As Catalin
pointed out, using "xxdiff -U" to work on a file with conflict
markers is less powerful than working on three stages directly,
but on the other hand it can be used as the last stage fixup,
independent from what git-merge does internally.  In other
words, it is meant to help solving the same problem but in a
different part of the workflow.

^ permalink raw reply

* Be leaner and slimmer by next week!
From: Orlando @ 2006-08-23 19:42 UTC (permalink / raw)
  To: bk-commits-head

Yo Bk-commits-head!!!



Amazing weight loss stories here:


http://www.domseta.com/n/?52&crosspointdewar




I've always had trouble with my weight ever since I was young. Of course I tried all the "best" fat loss products, nothing helped very much. It wasn't til I tried Anatrim that I saw the pounds seriously start to melt away! Nothing helped me lose weight faster. I literally saw 15 pounds melt away within the first few weeks! There's nothing more exciting than watching pounds disappear, especially when you've tried all sorts of different methods and products before. I've since read up on Anatrim and am amazed at the number of people who have benefited from its amazing results. I'm halfway to my goal, Anatrim will get me the rest of the way ;)


Jordan McKenzie, New York


http://www.obesor.net/hd/?52&diamagneticcadent

^ permalink raw reply

* Re: Proposal for new git Merge Strategy
From: Jakub Narebski @ 2006-08-23 19:02 UTC (permalink / raw)
  To: git
In-Reply-To: <E1GFxeZ-0000Nw-ED@jdl.com>

Jon Loeliger wrote:

> The other day, I was talking to some other folks else-list
> about git's approach to merges and mentioned that there was
> some structure in place to handle different merge strategies.
> 
> One person observed that Perforce had a really good
> approach to merging and conflict resolution that allowed
> user interaction during the process specifically to
> help select the individual files and hunks that contributed
> to the final result.  I confess that I have never used
> Perforce, so this is all hear-say and interpretation. :-)
> 
> However, it does seem like an approach that we could
> easily add to git -- not as the default of course.
> (Just think how dead we'd all be if Linus had to manually
> interact with every merge he performed at the tip of the
> Linux Pyramid. :-)
> 
> But for complex or critical merges, a "guided merge"
> strategy seems like it might be a useful tool.  Basically,
> it would offer options to select Stage 1 or Stage 2
> revisions, or step in and offer hunks from Stage 1 and 2,
> revert to "ours" or "theirs", or "revert to 'ours' or 'theirs'
> for all remaining files".  Things like that maybe.

And select which files are which (after renaming, copying, etc.)

> Any thoughts down this line?  Good idea?  Bad idea?

Wouldn't it be better to fallback to graphical/user guided 
merger only on _failed_ merge? Merge helpers like xxdiff, 
Meld or KDiff3 
  http://git.or.cz/gitwiki/InterfacesFrontendsAndToolsWishlist
There was proposal of git script which run xxdiff with correct
extracted files, if I remeber correctly postponed waiting for
more generic version.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* [PATCH] git-daemon virtual hosting implementation.
From: Pierre Habouzit @ 2006-08-23 18:52 UTC (permalink / raw)
  To: git; +Cc: Pierre Habouzit

just add the hostname in the path when using --base-path and --user-path.
this should be enough for most needs.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 Here is a proposal for daemon side virtualhosting support.

 This is quite simple, and just uses the hostname in the path where it looks
 for the git repository. Only works in conjuction of --base-path or
 --user-path btw.

 Documentation/git-daemon.txt |   12 +++++++
 daemon.c                     |   73 +++++++++++++++++++++++++++++++++++-------
 2 files changed, 73 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index 0f7d274..ab5b232 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -79,6 +79,18 @@ OPTIONS
 	taken as a request to access `path/foo` repository in
 	the home directory of user `alice`.
 
+--use-vhosts::
+	Use git daemon cheap virtual hosting scheme.
++
+Used with --base-path=/foo, it will search the repositories in /foo/HOSTNAME
+instead of /foo - if you try to pull git://git.example.com/hello.git, it will
+search into /foo/git.example.com/hello.git.
++
+When using --user-path the HOSTNAME is appended to the path as well, so for
+--user-path=some/path, request to git://git.example.com/~alice/foo is taken as
+a request to access some/path/git.example.com/foo in the home directory of
+user `alice`.
+
 --verbose::
 	Log details about the incoming connections and requested files.
 
diff --git a/daemon.c b/daemon.c
index 012936f..146482c 100644
--- a/daemon.c
+++ b/daemon.c
@@ -17,7 +17,7 @@ static int reuseaddr;
 
 static const char daemon_usage[] =
 "git-daemon [--verbose] [--syslog] [--inetd | --port=n] [--export-all]\n"
-"           [--timeout=n] [--init-timeout=n] [--strict-paths]\n"
+"           [--timeout=n] [--init-timeout=n] [--use-vhosts] [--strict-paths]\n"
 "           [--base-path=path] [--user-path | --user-path=path]\n"
 "           [--reuseaddr] [--detach] [--pid-file=file] [directory...]";
 
@@ -25,6 +25,8 @@ static const char daemon_usage[] =
 static char **ok_paths;
 static int strict_paths;
 
+static int use_vhosts;
+
 /* If this is set, git-daemon-export-ok is not required */
 static int export_all_trees;
 
@@ -148,7 +150,7 @@ static int avoid_alias(char *p)
 	}
 }
 
-static char *path_ok(char *dir)
+static char *path_ok(char *dir, char *vhost)
 {
 	static char rpath[PATH_MAX];
 	char *path;
@@ -158,6 +160,11 @@ static char *path_ok(char *dir)
 		return NULL;
 	}
 
+	if (use_vhosts && !vhost) {
+		logerror("using virtual hosting, and not host= was specified !");
+		return NULL;
+	}
+
 	if (*dir == '~') {
 		if (!user_path) {
 			logerror("'%s': User-path not allowed", dir);
@@ -174,9 +181,25 @@ static char *path_ok(char *dir)
 				slash = dir + restlen;
 			namlen = slash - dir;
 			restlen -= namlen;
-			loginfo("userpath <%s>, request <%s>, namlen %d, restlen %d, slash <%s>", user_path, dir, namlen, restlen, slash);
-			snprintf(rpath, PATH_MAX, "%.*s/%s%.*s",
-				 namlen, dir, user_path, restlen, slash);
+
+			if (use_vhosts) {
+				loginfo("host <%s>, "
+					"userpath <%s>, request <%s>, "
+					"namlen %d, restlen %d, slash <%s>",
+					vhost,
+					user_path, dir,
+					namlen, restlen, slash);
+				snprintf(rpath, PATH_MAX, "%.*s/%s/%s%.*s",
+					 namlen, dir, user_path, vhost,
+					 restlen, slash);
+			} else {
+				loginfo("userpath <%s>, request <%s>, "
+					"namlen %d, restlen %d, slash <%s>",
+					user_path, dir,
+					namlen, restlen, slash);
+				snprintf(rpath, PATH_MAX, "%.*s/%s%.*s",
+					 namlen, dir, user_path, restlen, slash);
+			}
 			dir = rpath;
 		}
 	}
@@ -187,7 +210,11 @@ static char *path_ok(char *dir)
 			return NULL;
 		}
 		else {
-			snprintf(rpath, PATH_MAX, "%s%s", base_path, dir);
+			if (use_vhosts) {
+				snprintf(rpath, PATH_MAX, "%s/%s%s", base_path, vhost, dir);
+			} else {
+				snprintf(rpath, PATH_MAX, "%s%s", base_path, dir);
+			}
 			dir = rpath;
 		}
 	}
@@ -229,15 +256,19 @@ static char *path_ok(char *dir)
 	return NULL;		/* Fallthrough. Deny by default */
 }
 
-static int upload(char *dir)
+static int upload(char *dir, char *vhost)
 {
 	/* Timeout as string */
 	char timeout_buf[64];
 	const char *path;
 
-	loginfo("Request for '%s'", dir);
+	if (vhost) {
+		loginfo("Request for '%s' (host = %s)", dir, vhost);
+	} else {
+		loginfo("Request for '%s'", dir);
+	}
 
-	if (!(path = path_ok(dir)))
+	if (!(path = path_ok(dir, vhost)))
 		return -1;
 
 	/*
@@ -274,6 +305,7 @@ static int execute(struct sockaddr *addr
 {
 	static char line[1000];
 	int pktlen, len;
+        char *vhost = NULL;
 
 	if (addr) {
 		char addrbuf[256] = "";
@@ -303,15 +335,28 @@ #endif
 	alarm(0);
 
 	len = strlen(line);
-	if (pktlen != len)
+	if (pktlen != len) {
+		int arg_pos = len + 1;
+
 		loginfo("Extended attributes (%d bytes) exist <%.*s>",
 			(int) pktlen - len,
-			(int) pktlen - len, line + len + 1);
+			(int) pktlen - len, line + arg_pos);
+
+		while (arg_pos < pktlen) {
+			int arg_len = strlen(line + arg_pos);
+
+			if (!strncmp("host=", line + arg_pos, 5)) {
+				vhost = line + arg_pos + 5;
+			}
+
+			arg_pos += arg_len + 1;
+		}
+	}
 	if (len && line[len-1] == '\n')
 		line[--len] = 0;
 
 	if (!strncmp("git-upload-pack ", line, 16))
-		return upload(line+16);
+		return upload(line+16, vhost);
 
 	logerror("Protocol error: '%s'", line);
 	return -1;
@@ -762,6 +807,10 @@ int main(int argc, char **argv)
 			init_timeout = atoi(arg+15);
 			continue;
 		}
+		if (!strcmp(arg, "--use-vhosts")) {
+			use_vhosts = 1;
+			continue;
+		}
 		if (!strcmp(arg, "--strict-paths")) {
 			strict_paths = 1;
 			continue;
-- 
1.4.1.1

^ permalink raw reply related

* Proposal for new git Merge Strategy
From: Jon Loeliger @ 2006-08-23 18:40 UTC (permalink / raw)
  To: git; +Cc: mwm

Folks,

The other day, I was talking to some other folks else-list
about git's approach to merges and mentioned that there was
some structure in place to handle different merge strategies.

One person observed that Perforce had a really good
approach to merging and conflict resolution that allowed
user interaction during the process specifically to
help select the individual files and hunks that contributed
to the final result.  I confess that I have never used
Perforce, so this is all hear-say and interpretation. :-)

However, it does seem like an approach that we could
easily add to git -- not as the default of course.
(Just think how dead we'd all be if Linus had to manually
interact with every merge he performed at the tip of the
Linux Pyramid. :-)

But for complex or critical merges, a "guided merge"
strategy seems like it might be a useful tool.  Basically,
it would offer options to select Stage 1 or Stage 2
revisions, or step in and offer hunks from Stage 1 and 2,
revert to "ours" or "theirs", or "revert to 'ours' or 'theirs'
for all remaining files".  Things like that maybe.

Any thoughts down this line?  Good idea?  Bad idea?

Thanks,
jdl

PS -- Please keep mwm on the CC: list as he doesn't
      directly subscribe to the git list, but would
      like to participate in the thread.  Thanks!

^ permalink raw reply

* Re: [PATCH] Added support for dropping privileges to git-daemon.
From: Tilman Sauerbeck @ 2006-08-23 16:45 UTC (permalink / raw)
  To: git
In-Reply-To: <7virkkl4ph.fsf@assigned-by-dhcp.cox.net>

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

Junio C Hamano [2006-08-22 16:40]:
> Tilman Sauerbeck <tilman@code-monkey.de> writes:
> 
> > +	These two options may be used to make `git-daemon` change its uid and
> > +	gid	before entering the server loop.
> > +	The uid that's used is the one of 'user'. If `group` is specified,
> > +	the gid is set to the one of 'group', otherwise, the default gid
> > +	of 'user' is used.
> 
> Funny whitespaces all over the place...

There's exactly _one_ stray \t in the middle of a line. Not quite what
I'd call "all over the place' ;)
I should probably tell vim to hilight space errors in txt files, too.

> Gaah again.  These options do not have any effect (other than
> sanity checking) on the inetd_mode codepath, so instead of
> saying this in the documentation I would suggest specifying
> these options an error under --inetd.

Good point.

Regards,
Tilman

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

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

^ permalink raw reply

* Re: git 1.4.2 RPMS for i386 on kernel.org
From: Seth Falcon @ 2006-08-23 16:12 UTC (permalink / raw)
  To: git
In-Reply-To: <ecd7o8$a9d$1@sea.gmane.org>

Jakub Narebski <jnareb@gmail.com> writes:

> Could anyone please generate RPMS for i386 architecture for git version
> 1.4.2 on kernel.org?
>   http://www.kernel.org/pub/software/scm/git/RPMS/i386/

Also, assuming 1.4.2 is actually released, the webpage needs
updating.  The link to the latest sources is 1.4.1.1:

http://git.or.cz/

+ seth

^ permalink raw reply

* Re: git cherry-pick feature request
From: Jeff King @ 2006-08-23 14:35 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: Junio C Hamano, git
In-Reply-To: <17644.21269.128308.313284@cargo.ozlabs.ibm.com>

On Wed, Aug 23, 2006 at 11:07:33PM +1000, Paul Mackerras wrote:

> While I'm asking for features, another one that would be really useful
> for another tool I'm writing is a 3-way diff for a file between the
> working directory, the index, and the current head commit, something
> like what git diff-tree -c does for merges.  That is, it would have

Theoretically I'm working on this, but I haven't really had time to get
started on it yet this week. If you have patience, I'll get to it. If
not, then somebody else is welcome to take a crack at it.

-Peff

^ permalink raw reply

* Re: [PATCH] Fix a comparison bug in diff-delta.c
From: Pierre Habouzit @ 2006-08-23 14:31 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0608231542380.28360@wbgn013.biozentrum.uni-wuerzburg.de>

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

Le mer 23 août 2006 15:45, Johannes Schindelin a écrit :
> Hi,
>
> On Wed, 23 Aug 2006, Pierre Habouzit wrote:
> > -	for (i = 4; (1 << i) < hsize && i < 31; i++);
> > +	for (i = 4; (1u << i) < hsize && i < 31; i++);
>
> The variable i never takes on the value 31 (or any higher value), so
> there is no bug here. Unless you port git to a system where an int
> has less than 32 bit.

that remains quite tasteless though, and maybe that patch can be 
integrated to the "cleanup" series I sent later today.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ permalink raw reply

* Re: [PATCH] Fix a comparison bug in diff-delta.c
From: Johannes Schindelin @ 2006-08-23 13:45 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git
In-Reply-To: <1156324675415-git-send-email-madcoder@debian.org>

Hi,

On Wed, 23 Aug 2006, Pierre Habouzit wrote:

> -	for (i = 4; (1 << i) < hsize && i < 31; i++);
> +	for (i = 4; (1u << i) < hsize && i < 31; i++);

The variable i never takes on the value 31 (or any higher value), so there 
is no bug here. Unless you port git to a system where an int has less than 
32 bit.

Ciao,
Dscho

^ permalink raw reply

* Re: git cherry-pick feature request
From: Paul Mackerras @ 2006-08-23 13:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfyfnixv5.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano writes:

> Quiet sounds sane.
> 
> Reverting to its "previous state" I am not quite sure if it is
> worth making it an option and run it as part of cherry-pick.

OK.  If the git cherry-pick terminates with an error I'll get gitk to
show the error message (if any) it printed along with a warning that
the working directory might need to be cleaned up.  I don't really
want to do a git reset automagically since the user might have local
modifications that they want to preserve.

While I'm asking for features, another one that would be really useful
for another tool I'm writing is a 3-way diff for a file between the
working directory, the index, and the current head commit, something
like what git diff-tree -c does for merges.  That is, it would have
two columns of +/-/space characters, one for the current head and one
for the index.  A '-' would indicate that the line appears in the
current head or the index but not in the version of the file in the
working directory.  A '+' would indicate that the line appears in the
working directory version.

How hard would it be to adapt the git diff-tree -c mechanism to work
on two blobs plus the file from the working directory?

Thanks,
Paul.

^ permalink raw reply

* [PATCH 5/7] missing #define DEBUG 0 that made the preprocessor whine
From: Pierre Habouzit @ 2006-08-23 10:39 UTC (permalink / raw)
  To: git; +Cc: Pierre Habouzit
In-Reply-To: <11563295573215-git-send-email-madcoder@debian.org>

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-grep.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index 0bd517b..3123494 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -14,6 +14,8 @@ #include <regex.h>
 #include <fnmatch.h>
 #include <sys/wait.h>
 
+#define DEBUG 0
+
 /*
  * git grep pathspecs are somewhat different from diff-tree pathspecs;
  * pathname wildcards are allowed.
-- 
1.4.1.1

^ permalink raw reply related

* [PATCH 3/7] missing 'static' keywords
From: Pierre Habouzit @ 2006-08-23 10:39 UTC (permalink / raw)
  To: git; +Cc: Pierre Habouzit
In-Reply-To: <11563295562422-git-send-email-madcoder@debian.org>

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-tar-tree.c |    2 +-
 http-push.c        |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c
index e0bcb0a..61a4135 100644
--- a/builtin-tar-tree.c
+++ b/builtin-tar-tree.c
@@ -275,7 +275,7 @@ static void traverse_tree(struct tree_de
 	}
 }
 
-int git_tar_config(const char *var, const char *value)
+static int git_tar_config(const char *var, const char *value)
 {
 	if (!strcmp(var, "tar.umask")) {
 		if (!strcmp(value, "user")) {
diff --git a/http-push.c b/http-push.c
index 4849779..7d12f69 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1700,7 +1700,7 @@ static int locking_available(void)
 	return lock_flags;
 }
 
-struct object_list **add_one_object(struct object *obj, struct object_list **p)
+static struct object_list **add_one_object(struct object *obj, struct object_list **p)
 {
 	struct object_list *entry = xmalloc(sizeof(struct object_list));
 	entry->item = obj;
-- 
1.4.1.1

^ permalink raw reply related

* [PATCH 6/7] use name[len] in switch directly, instead of creating a shadowed variable.
From: Pierre Habouzit @ 2006-08-23 10:39 UTC (permalink / raw)
  To: git; +Cc: Pierre Habouzit
In-Reply-To: <11563295573035-git-send-email-madcoder@debian.org>

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-apply.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index 5991737..f8f5eeb 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -606,9 +606,7 @@ static char *git_header_name(char *line,
 	 * form.
 	 */
 	for (len = 0 ; ; len++) {
-		char c = name[len];
-
-		switch (c) {
+		switch (name[len]) {
 		default:
 			continue;
 		case '\n':
-- 
1.4.1.1

^ permalink raw reply related

* [PATCH 1/7] avoid to use error that shadows the function name, use err instead.
From: Pierre Habouzit @ 2006-08-23 10:39 UTC (permalink / raw)
  To: git; +Cc: Pierre Habouzit

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-apply.c |    6 +++---
 builtin-push.c  |   10 +++++-----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index 4f0eef0..5991737 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1907,13 +1907,13 @@ static int check_patch(struct patch *pat
 static int check_patch_list(struct patch *patch)
 {
 	struct patch *prev_patch = NULL;
-	int error = 0;
+	int err = 0;
 
 	for (prev_patch = NULL; patch ; patch = patch->next) {
-		error |= check_patch(patch, prev_patch);
+		err |= check_patch(patch, prev_patch);
 		prev_patch = patch;
 	}
-	return error;
+	return err;
 }
 
 static void show_index_list(struct patch *list)
diff --git a/builtin-push.c b/builtin-push.c
index 2b5e6fa..ada8338 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -232,7 +232,7 @@ static int do_push(const char *repo)
 	common_argc = argc;
 
 	for (i = 0; i < n; i++) {
-		int error;
+		int err;
 		int dest_argc = common_argc;
 		int dest_refspec_nr = refspec_nr;
 		const char **dest_refspec = refspec;
@@ -248,10 +248,10 @@ static int do_push(const char *repo)
 		while (dest_refspec_nr--)
 			argv[dest_argc++] = *dest_refspec++;
 		argv[dest_argc] = NULL;
-		error = run_command_v(argc, argv);
-		if (!error)
+		err = run_command_v(argc, argv);
+		if (!err)
 			continue;
-		switch (error) {
+		switch (err) {
 		case -ERR_RUN_COMMAND_FORK:
 			die("unable to fork for %s", sender);
 		case -ERR_RUN_COMMAND_EXEC:
@@ -262,7 +262,7 @@ static int do_push(const char *repo)
 		case -ERR_RUN_COMMAND_WAITPID_NOEXIT:
 			die("%s died with strange error", sender);
 		default:
-			return -error;
+			return -err;
 		}
 	}
 	return 0;
-- 
1.4.1.1

^ permalink raw reply related

* [PATCH 2/7] git_dir holds pointers to local strings, hence MUST be const.
From: Pierre Habouzit @ 2006-08-23 10:39 UTC (permalink / raw)
  To: git; +Cc: Pierre Habouzit
In-Reply-To: <11563295562072-git-send-email-madcoder@debian.org>

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 cache.h       |    2 +-
 environment.c |    7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/cache.h b/cache.h
index 08d6a91..3044794 100644
--- a/cache.h
+++ b/cache.h
@@ -123,7 +123,7 @@ #define DB_ENVIRONMENT "GIT_OBJECT_DIREC
 #define INDEX_ENVIRONMENT "GIT_INDEX_FILE"
 #define GRAFT_ENVIRONMENT "GIT_GRAFT_FILE"
 
-extern char *get_git_dir(void);
+extern const char *get_git_dir(void);
 extern char *get_object_directory(void);
 extern char *get_refs_directory(void);
 extern char *get_index_file(void);
diff --git a/environment.c b/environment.c
index e6bd003..5fae9ac 100644
--- a/environment.c
+++ b/environment.c
@@ -25,8 +25,9 @@ int zlib_compression_level = Z_DEFAULT_C
 int pager_in_use;
 int pager_use_color = 1;
 
-static char *git_dir, *git_object_dir, *git_index_file, *git_refs_dir,
-	*git_graft_file;
+static const char *git_dir;
+static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;
+
 static void setup_git_env(void)
 {
 	git_dir = getenv(GIT_DIR_ENVIRONMENT);
@@ -49,7 +50,7 @@ static void setup_git_env(void)
 		git_graft_file = strdup(git_path("info/grafts"));
 }
 
-char *get_git_dir(void)
+const char *get_git_dir(void)
 {
 	if (!git_dir)
 		setup_git_env();
-- 
1.4.1.1

^ permalink raw reply related

* [PATCH 4/7] remove ugly shadowing of loop indexes in subloops.
From: Pierre Habouzit @ 2006-08-23 10:39 UTC (permalink / raw)
  To: git; +Cc: Pierre Habouzit
In-Reply-To: <1156329556788-git-send-email-madcoder@debian.org>

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-mv.c |    6 +++---
 git.c        |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin-mv.c b/builtin-mv.c
index ff882be..fd1e520 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -262,10 +262,10 @@ int cmd_mv(int argc, const char **argv, 
 	} else {
 		for (i = 0; i < changed.nr; i++) {
 			const char *path = changed.items[i].path;
-			int i = cache_name_pos(path, strlen(path));
-			struct cache_entry *ce = active_cache[i];
+			int j = cache_name_pos(path, strlen(path));
+			struct cache_entry *ce = active_cache[j];
 
-			if (i < 0)
+			if (j < 0)
 				die ("Huh? Cache entry for %s unknown?", path);
 			refresh_cache_entry(ce, 0);
 		}
diff --git a/git.c b/git.c
index 930998b..a01d195 100644
--- a/git.c
+++ b/git.c
@@ -292,11 +292,11 @@ static void handle_internal_command(int 
 		if (p->option & USE_PAGER)
 			setup_pager();
 		if (getenv("GIT_TRACE")) {
-			int i;
+			int j;
 			fprintf(stderr, "trace: built-in: git");
-			for (i = 0; i < argc; ++i) {
+			for (j = 0; j < argc; ++j) {
 				fputc(' ', stderr);
-				sq_quote_print(stderr, argv[i]);
+				sq_quote_print(stderr, argv[j]);
 			}
 			putc('\n', stderr);
 			fflush(stderr);
-- 
1.4.1.1

^ permalink raw reply related

* [PATCH 7/7] n is in fact unused, and is later shadowed.
From: Pierre Habouzit @ 2006-08-23 10:39 UTC (permalink / raw)
  To: git; +Cc: Pierre Habouzit
In-Reply-To: <1156329557424-git-send-email-madcoder@debian.org>

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 date.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/date.c b/date.c
index 66be23a..63d1d5b 100644
--- a/date.c
+++ b/date.c
@@ -584,10 +584,9 @@ static const char *approxidate_alpha(con
 	const struct typelen *tl;
 	const struct special *s;
 	const char *end = date;
-	int n = 1, i;
+	int i;
 
-	while (isalpha(*++end))
-		n++;
+	while (isalpha(*++end));
 
 	for (i = 0; i < 12; i++) {
 		int match = match_string(date, month_names[i]);
-- 
1.4.1.1

^ permalink raw reply related

* [PATCH 0/7] C cleanup series, mostly nasty shadowing
From: Pierre Habouzit @ 2006-08-23 10:38 UTC (permalink / raw)
  To: git

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


This series of patches intend to fix some not very nice bits of code.
None of them are critical though.

 0001-avoid-to-use-error-that-shadows-the-function-name-use-err-instead.txt
 0002-git_dir-holds-pointers-to-local-strings-hence-MUST-be-const.txt
 0003-missing-static-keywords.txt
 0004-remove-ugly-shadowing-of-loop-indexes-in-subloops.txt
 0005-missing-define-DEBUG-0-that-made-the-preprocessor-whine.txt
 0006-use-name-len-in-switch-directly-instead-of-creating-a-shadowed-variable.txt
 0007-n-is-in-fact-unused-and-is-later-shadowed.txt

 builtin-apply.c    |   10 ++++------
 builtin-grep.c     |    2 ++
 builtin-mv.c       |    6 +++---
 builtin-push.c     |   10 +++++-----
 builtin-tar-tree.c |    2 +-
 cache.h            |    2 +-
 date.c             |    5 ++---
 environment.c      |    7 ++++---
 git.c              |    6 +++---
 http-push.c        |    2 +-
 10 files changed, 26 insertions(+), 26 deletions(-)

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ permalink raw reply

* "cg-commit -M msg-file ..." fails when not run from top of tree
From: Jim Meyering @ 2006-08-23 10:27 UTC (permalink / raw)
  To: git

Hello,

I discovered that "cg-commit -M MSG-FILE ..." fails when run from
a subdirectory, and when MSG-FILE is a relative file name.
This is using cogito-0.17.3-2 from Debian/unstable, but the problem
remains when using the latest cogito sources, pulled minutes ago.

Here's an example:

  $ mkdir a; touch a/x; cg-init -m. .; cd a; echo . > x; cg-commit -M x x
  defaulting to local storage area
  Adding file a/x
  Committing initial tree 341d89829a1bf9c0ccfbccf738815cbc862b3242
  Committed as 6497164c6f8e86220ff26c6b89b9d0dbad5a7743
  cat: x: No such file or directory
  [Exit 1]

This appears to be due to the "cd", that can happen in cg-Xlib:

  _git="${GIT_DIR:-.git}"
  if [ ! "$_git_repo_unneeded" ] && [ ! "$GIT_DIR" ] && [ ! -d "$_git" ]; then
          _git_abs_path="$(git-rev-parse --git-dir 2>/dev/null)"
          if [ -d "$_git_abs_path" ]; then
                  _git_relpath="$(git-rev-parse --show-prefix)"
==========>       cd "$_git_abs_path/.."         <==============
          fi
  fi
  _git_objects="${GIT_OBJECT_DIRECTORY:-$_git/objects}"

I can work around the problem by using an absolute name for
the message file, but I shouldn't have to do that.

FWIW, I tried setting GIT_DIR to the absolute name of the .git directory,
but that just made it so cg-commit failed with this diagnostic:

  cg-commit: Nothing to commit

Jim

^ permalink raw reply

* Re: [PATCH 1/3] gitweb: Whitespace cleanup: realign, reindent
From: Junio C Hamano @ 2006-08-23  9:55 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <ech55l$reh$1@sea.gmane.org>

Jakub Narebski <jnareb@gmail.com> writes:

> So what is the default? 5-column tabs? 8-column tabs? And to what width?
> 80-column wide? 120-column wide?

Quoting the person who invented git:

                    Chapter 1: Indentation

    Tabs are 8 characters, and thus indentations are also 8 characters.
    There are heretic movements that try to make indentations 4 (or even 2!)
    characters deep, and that is akin to trying to define the value of PI to
    be 3.

    Rationale: The whole idea behind indentation is to clearly define where
    a block of control starts and ends.  Especially when you've been looking
    at your screen for 20 straight hours, you'll find it a lot easier to see
    how the indentation works if you have large indentations.

    Now, some people will claim that having 8-character indentations makes
    the code move too far to the right, and makes it hard to read on a
    80-character terminal screen.  The answer to that is that if you need
    more than 3 levels of indentation, you're screwed anyway, and should fix
    your program.

^ permalink raw reply

* Re: git cherry-pick feature request
From: Junio C Hamano @ 2006-08-23  9:51 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git
In-Reply-To: <17643.62896.396783.890223@cargo.ozlabs.ibm.com>

Paul Mackerras <paulus@samba.org> writes:

> Could I have a flag to git cherry-pick (-q for quiet, maybe) that
> tells it not to print anything if the command succeeds?  Could I also
> have a flag that tells it to clean up if the merge fails and leave the
> tree in its previous state?

Quiet sounds sane.

Reverting to its "previous state" I am not quite sure if it is
worth making it an option and run it as part of cherry-pick.

^ 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