Git development
 help / color / mirror / Atom feed
* [PATCH] gitweb: Remove redundant $searchtype setup
From: Petr Baudis @ 2007-05-18 23:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Sorry, this was inadverently introduced by my grep search patch. It causes
annoying "redefined" warnings.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ac78a10..0143183 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -413,13 +413,6 @@ if (defined $searchtext) {
 	$search_regexp = quotemeta $searchtext;
 }
 
-our $searchtype = $cgi->param('st');
-if (defined $searchtype) {
-	if ($searchtype =~ m/[^a-z]/) {
-		die_error(undef, "Invalid searchtype parameter");
-	}
-}
-
 # now read PATH_INFO and use it as alternative to parameters
 sub evaluate_path_info {
 	return if defined $project;

^ permalink raw reply related

* [PATCH 3/3] gitweb: Remove git_blame (superseded by git_blame2)
From: Petr Baudis @ 2007-05-18 23:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Petr Baudis
In-Reply-To: <20070518230227.17359.16402.stgit@rover>

This patch definitely removes git_blame() from the source and renames
git_blame2() to git_blame(); it was already the default handler for the
blame action for a long time and it has been actually broken for some time
now (I'm not sure how long), so noone probably cares about it much (I have
an alternative trivial patch to fix it too). All the information listing is
already included in git_blame2() output now.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f301718..c7a3fda 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -473,7 +473,7 @@ $git_dir = "$projectroot/$project" if $project;
 
 # dispatch
 my %actions = (
-	"blame" => \&git_blame2,
+	"blame" => \&git_blame,
 	"blame_incremental" => \&git_blame_incremental,
 	"blame_data" => \&git_blame_data,
 	"blobdiff" => \&git_blobdiff,
@@ -3777,105 +3777,8 @@ sub git_blame_incremental {
 	git_blame_common('incremental');
 }
 
-sub git_blame2 {
-	git_blame_common('oneshot');
-}
-
 sub git_blame {
-	my $fd;
-
-	my ($have_blame) = gitweb_check_feature('blame');
-	if (!$have_blame) {
-		die_error('403 Permission denied', "Permission denied");
-	}
-	die_error('404 Not Found', "File name not defined") if (!$file_name);
-	$hash_base ||= git_get_head_hash($project);
-	die_error(undef, "Couldn't find base commit") unless ($hash_base);
-	my %co = parse_commit($hash_base)
-		or die_error(undef, "Reading commit failed");
-	if (!defined $hash) {
-		$hash = git_get_hash_by_path($hash_base, $file_name, "blob")
-			or die_error(undef, "Error lookup file");
-	}
-	open ($fd, "-|", git_cmd(), "annotate", '-l', '-t', '-r', $file_name, $hash_base)
-		or die_error(undef, "Open git-annotate failed");
-	git_header_html();
-	my $formats_nav =
-		$cgi->a({-href => href(action=>"blob", hash=>$hash, hash_base=>$hash_base, file_name=>$file_name)},
-		        "blob") .
-		" | " .
-		$cgi->a({-href => href(action=>"history", hash=>$hash, hash_base=>$hash_base, file_name=>$file_name)},
-			"history") .
-		" | " .
-		$cgi->a({-href => href(action=>"blame", file_name=>$file_name), -class => "blamelink"},
-		        "HEAD");
-	git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
-	git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
-	git_print_page_path($file_name, 'blob', $hash_base);
-	print "<div class=\"page_body\">\n";
-	print <<HTML;
-<table class="blame">
-  <tr>
-    <th>Commit</th>
-    <th>Age</th>
-    <th>Author</th>
-    <th>Line</th>
-    <th>Data</th>
-  </tr>
-HTML
-	my @line_class = (qw(light dark));
-	my $line_class_len = scalar (@line_class);
-	my $line_class_num = $#line_class;
-	while (my $line = <$fd>) {
-		my $long_rev;
-		my $short_rev;
-		my $author;
-		my $time;
-		my $lineno;
-		my $data;
-		my $age;
-		my $age_str;
-		my $age_class;
-
-		chomp $line;
-		$line_class_num = ($line_class_num + 1) % $line_class_len;
-
-		if ($line =~ m/^([0-9a-fA-F]{40})\t\(\s*([^\t]+)\t(\d+) [+-]\d\d\d\d\t(\d+)\)(.*)$/) {
-			$long_rev = $1;
-			$author   = $2;
-			$time     = $3;
-			$lineno   = $4;
-			$data     = $5;
-		} else {
-			print qq(  <tr><td colspan="5" class="error">Unable to parse: $line</td></tr>\n);
-			next;
-		}
-		$short_rev  = substr ($long_rev, 0, 8);
-		$age        = time () - $time;
-		$age_str    = age_string ($age);
-		$age_str    =~ s/ /&nbsp;/g;
-		$age_class  = age_class($age);
-		$author     = esc_html ($author);
-		$author     =~ s/ /&nbsp;/g;
-
-		$data = untabify($data);
-		$data = esc_html ($data);
-
-		print <<HTML;
-  <tr class="$line_class[$line_class_num]">
-    <td class="sha1"><a href="${\href (action=>"commit", hash=>$long_rev)}" class="text">$short_rev..</a></td>
-    <td class="$age_class">$age_str</td>
-    <td>$author</td>
-    <td class="linenr"><a id="$lineno" href="#$lineno" class="linenr">$lineno</a></td>
-    <td class="pre">$data</td>
-  </tr>
-HTML
-	} # while (my $line = <$fd>)
-	print "</table>\n\n";
-	close $fd
-		or print "Reading blob failed.\n";
-	print "</div>";
-	git_footer_html();
+	git_blame_common('oneshot');
 }
 
 sub git_tags {

^ permalink raw reply related

* [PATCH 2/3] gitweb: Extra columns in blame
From: Petr Baudis @ 2007-05-18 23:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Petr Baudis
In-Reply-To: <20070518230227.17359.16402.stgit@rover>

This patch adds extra columns to blame output, containing
line author and creation date. These columns are by default hidden by
display: none but by clicking on the expansion "button" you can display
them (and hide again). I think seeing this information without tooltips
fishing can give much better overview of the content evolution.

This patch depends on the interactive blame patch (but can be factored out;
the common required parts are just the blame.js infrastructure).

Signed-off-by: Petr Baudis <pasky@suse.cz>

---

This version is updated for the new version of incremental blame and
features updated javascript code based on a friend's suggestions that
should improve MSIE compatibility and performance.
---

 gitweb/blame.js    |   48 +++++++++++++++++++++++++++++++++++++++++++++++-
 gitweb/gitweb.css  |    5 +++++
 gitweb/gitweb.perl |   17 +++++++++++++----
 3 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/gitweb/blame.js b/gitweb/blame.js
index bd51275..f6d661a 100644
--- a/gitweb/blame.js
+++ b/gitweb/blame.js
@@ -1,4 +1,44 @@
 // Copyright (C) 2007, Fredrik Kuivinen <frekui@gmail.com>
+// Copyright (C) 2007, Petr Baudis <pasky@suse.cz>
+
+
+// blame extra columns
+
+// I would like to note here that JavaScript is utterly stupid.
+function findStyleRule(styleName) {
+	for (i = 0; i < document.styleSheets.length; i++) { 
+		// MSIE has .rules, Mozilla has .cssRules
+		var cssRules = document.styleSheets[i].cssRules ? document.styleSheets[i].cssRules : document.styleSheets[i].rules;
+		for (j = 0; j < cssRules.length; j++) {
+			var rule = cssRules[j];
+			if (rule.selectorText.toLowerCase() == styleName) {
+				return rule;
+			}
+		}
+	}
+}
+
+var isIE = (navigator.appName.toLowerCase().indexOf("microsoft") != -1);
+var extra_columns = 0;
+var extra_column_rule = null;
+function extra_blame_columns() {
+	if (!extra_column_rule)
+		extra_column_rule = findStyleRule(".extra_column");
+
+	if (!extra_columns) {
+		document.getElementById("columns_expander").innerHTML = "[-]";
+		extra_column_rule.style.display = isIE ? "inline" : "table-cell";
+		extra_columns = 1;
+	} else {
+		document.getElementById("columns_expander").innerHTML = "[+]";
+		extra_column_rule.style.display = "none";
+		extra_columns = 0;
+	}
+}
+
+
+// blame_interactive
+
 
 var DEBUG = 0;
 function debug(str)
@@ -72,14 +112,20 @@ function handleLine(commit)
 			+ zeroPad(date.getUTCSeconds());
 		tr.firstChild.title = commit.author + ', ' + dateStr + ' ' + timeStr;
 		var shaAnchor = tr.firstChild.firstChild;
+		var authorField = tr.firstChild.nextSibling;
+		var dateField = tr.firstChild.nextSibling.nextSibling;
 		if (i == 0) {
 			shaAnchor.href = baseUrl + ';a=commit;h=' + commit.sha1;
 			shaAnchor.innerHTML = commit.sha1.substr(0, 8);
+			authorField.innerHTML = commit.author;
+			dateField.innerHTML = dateStr + ' ' + timeStr;
 		} else {
 			shaAnchor.innerHTML = '';
+			authorField.innerHTML = '';
+			dateField.innerHTML = '';
 		}
 
-		var lineAnchor = tr.firstChild.nextSibling.firstChild;
+		var lineAnchor = tr.firstChild.nextSibling.nextSibling.nextSibling.firstChild;
 		lineAnchor.href = baseUrl + ';a=blame;hb=' + commit.sha1
 			+ ';f=' + commit.filename + '#l' + commit.srcline;
 		resline++;
diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 9f0822f..48b50b0 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -488,3 +488,8 @@ span.match {
 div.binary {
 	font-style: italic;
 }
+
+/* This selector is hardcoded in gitweb.perl */
+.extra_column {
+	display: none;
+}
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index dae35de..f301718 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3677,11 +3677,16 @@ sub git_blame_common {
 	my $num_colors = scalar(@rev_color);
 	my $current_color = 0;
 	my $last_rev;
-	print "<script type=\"text/javascript\" src=\"$blamejs\"></script>\n" if $type eq 'incremental';
+	print "<script type=\"text/javascript\" src=\"$blamejs\"></script>\n";
 	print <<HTML;
+
 <div class="page_body">
 <table class="blame">
-<tr><th>Commit</th><th>Line</th><th>Data</th></tr>
+<tr><th>Commit&nbsp;<a href="javascript:extra_blame_columns()" id="columns_expander">[+]</a></th>
+<th class="extra_column">Author</th>
+<th class="extra_column">Date</th>
+<th>Line</th>
+<th>Data</th></tr>
 HTML
 	my %metainfo = ();
 	my $linenr = 0;
@@ -3692,6 +3697,8 @@ HTML
 			$linenr += 1;
 			print "<tr id=\"l$linenr\" class=\"light2\">";
 			print '<td class="sha1"><a href=""></a></td>';
+			print "<td class=\"extra_column\"></td>";
+			print "<td class=\"extra_column\"></td>";
 			print "<td class=\"linenr\"><a class=\"linenr\" href=\"\">$linenr</a></td><td class=\"pre\">" . esc_html($_) . "</td>\n";
 			print "</tr>\n";
 			next;
@@ -3721,15 +3728,17 @@ HTML
 		}
 		print "<tr class=\"$rev_color[$current_color]\">\n";
 		if ($group_size) {
+			my $rowspan = $group_size > 1 ? " rowspan=\"$group_size\"" : "";
 			print "<td class=\"sha1\"";
 			print " title=\"". esc_html($author) . ", $date\"";
-			print " rowspan=\"$group_size\"" if ($group_size > 1);
-			print ">";
+			print "$rowspan>";
 			print $cgi->a({-href => href(action=>"commit",
 			                             hash=>$full_rev,
 			                             file_name=>$file_name)},
 			              esc_html($rev));
 			print "</td>\n";
+			print "<td class=\"extra_column\" $rowspan>". esc_html($author) . "</td>";
+			print "<td class=\"extra_column\" $rowspan>". $date . "</td>";
 		}
 		open (my $dd, "-|", git_cmd(), "rev-parse", "$full_rev^")
 			or die_error(undef, "Open git-rev-parse failed");

^ permalink raw reply related

* [PATCH 1/3] gitweb: Incremental blame
From: Petr Baudis @ 2007-05-18 23:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Fredrik Kuivinen, Petr Baudis

This is tweaked up version of Fredrik Kuivinen <frekui@gmail.com>'s proof
of concept patch to add support for incrementally displaying line data in
the blame view using some javascript gadgetry.

The original patch has been lightly tested in a couple of browsers
(Firefox, Mozilla, Konqueror, Galeon, Opera and IE6). The new patch has
been tested in Firefox and Epiphany (and works fine in Epiphany 2.14.3,
contrary to what the original patch claimed).

Compared to the original patch, this one works with pathinfo-ish URLs as
well, and should play well with non-javascript browsers as well (the HTML
points to the blame action, while javascript code rewrites the links to use
the blame_incremental action; it is somewhat hackish but I couldn't think
of a better solution). Also, this version of the patch avoids duplicated
code with git_blame2(). blame.js indentation has been fixed.

As usual, you can see it in action at repo.or.cz. blame view now feels a
lot more friendly.

Cc: Fredrik Kuivinen <frekui@gmail.com>
Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 Makefile           |    6 +-
 git-instaweb.sh    |    7 ++
 gitweb/blame.js    |  193 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 gitweb/gitweb.perl |  123 ++++++++++++++++++++++++++++-----
 4 files changed, 310 insertions(+), 19 deletions(-)

diff --git a/Makefile b/Makefile
index ed12577..46a9fcb 100644
--- a/Makefile
+++ b/Makefile
@@ -168,6 +168,7 @@ GITWEB_HOMETEXT = indextext.html
 GITWEB_CSS = gitweb.css
 GITWEB_LOGO = git-logo.png
 GITWEB_FAVICON = git-favicon.png
+GITWEB_BLAMEJS = blame.js
 GITWEB_SITE_HEADER =
 GITWEB_SITE_FOOTER =
 
@@ -815,13 +816,14 @@ gitweb/gitweb.cgi: gitweb/gitweb.perl
 	    -e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \
 	    -e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \
 	    -e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \
+	    -e 's|++GITWEB_BLAMEJS++|$(GITWEB_BLAMEJS)|g' \
 	    -e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
 	    -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
 	    $< >$@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@
 
-git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css
+git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css gitweb/blame.js
 	$(QUIET_GEN)rm -f $@ $@+ && \
 	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
@@ -830,6 +832,8 @@ git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css
 	    -e '/@@GITWEB_CGI@@/d' \
 	    -e '/@@GITWEB_CSS@@/r gitweb/gitweb.css' \
 	    -e '/@@GITWEB_CSS@@/d' \
+	    -e '/@@GITWEB_BLAMEJS@@/r gitweb/blame.js' \
+	    -e '/@@GITWEB_BLAMEJS@@/d' \
 	    $@.sh > $@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@
diff --git a/git-instaweb.sh b/git-instaweb.sh
index cbc7418..dddbb6b 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -233,8 +233,15 @@ gitweb_css () {
 EOFGITWEB
 }
 
+gitweb_blamejs () {
+	cat > "$1" <<\EOFGITWEB
+@@GITWEB_BLAMEJS@@
+EOFGITWEB
+}
+
 gitweb_cgi $GIT_DIR/gitweb/gitweb.cgi
 gitweb_css $GIT_DIR/gitweb/gitweb.css
+gitweb_blamejs $GIT_DIR/gitweb/blame.js
 
 case "$httpd" in
 *lighttpd*)
diff --git a/gitweb/blame.js b/gitweb/blame.js
new file mode 100644
index 0000000..bd51275
--- /dev/null
+++ b/gitweb/blame.js
@@ -0,0 +1,193 @@
+// Copyright (C) 2007, Fredrik Kuivinen <frekui@gmail.com>
+
+var DEBUG = 0;
+function debug(str)
+{
+	if (DEBUG)
+		alert(str);
+}
+
+function createRequestObject() {
+	var ro;
+	if (window.XMLHttpRequest) {
+		ro = new XMLHttpRequest();
+	} else {
+		ro = new ActiveXObject("Microsoft.XMLHTTP");
+	}
+	return ro;
+}
+
+var http;
+var baseUrl;
+
+// 'commits' is an associative map. It maps SHA1s to Commit objects.
+var commits = new Object();
+
+function Commit(sha1)
+{
+	this.sha1 = sha1;
+}
+
+function zeroPad(n)
+{
+	if (n < 10)
+		return '0' + n;
+	else
+		return n.toString();
+}
+
+function handleLine(commit)
+{
+	/* This is the structure of the HTML fragment we are working
+	   with:
+	   
+	   <tr id="l123" class="light2">
+	   <td class="sha1" title="">
+	   <a href=""></a>
+	   </td>
+	   <td class="linenr">
+	   <a class="linenr" href="">123</a>
+	   </td>
+	   <td class="pre"># times (my ext3 doesn&#39;t).</td>
+	   </tr>
+	 */
+
+	var resline = commit.resline;
+	for (var i = 0; i < commit.numlines; i++) {
+		var tr = document.getElementById('l'+resline);
+		if (!tr) {
+			debug('tr is null! resline: ' + resline);
+			break;
+		}
+
+		var date = new Date();
+		date.setTime(commit.authorTime * 1000);
+		var dateStr =
+			date.getUTCFullYear() + '-'
+			+ zeroPad(date.getUTCMonth()+1) + '-'
+			+ zeroPad(date.getUTCDate());
+		var timeStr =
+			zeroPad(date.getUTCHours()) + ':'
+			+ zeroPad(date.getUTCMinutes()) + ':'
+			+ zeroPad(date.getUTCSeconds());
+		tr.firstChild.title = commit.author + ', ' + dateStr + ' ' + timeStr;
+		var shaAnchor = tr.firstChild.firstChild;
+		if (i == 0) {
+			shaAnchor.href = baseUrl + ';a=commit;h=' + commit.sha1;
+			shaAnchor.innerHTML = commit.sha1.substr(0, 8);
+		} else {
+			shaAnchor.innerHTML = '';
+		}
+
+		var lineAnchor = tr.firstChild.nextSibling.firstChild;
+		lineAnchor.href = baseUrl + ';a=blame;hb=' + commit.sha1
+			+ ';f=' + commit.filename + '#l' + commit.srcline;
+		resline++;
+	}
+}
+
+function fixColors()
+{
+	var colorClasses = ['light2', 'dark2'];
+	var linenum = 1;
+	var tr;
+	var colorClass = 0;
+
+	while ((tr = document.getElementById('l'+linenum))) {
+		if (tr.firstChild.firstChild.innerHTML != '') {
+			colorClass = (colorClass + 1) % 2;
+		}
+		tr.setAttribute('class', colorClasses[colorClass]);
+		// Internet Explorer needs this
+		tr.setAttribute('className', colorClasses[colorClass]);
+		linenum++;
+	}
+}
+
+var prevDataLength = -1;
+var nextLine = 0;
+var inProgress = false;
+
+var sha1Re = new RegExp('([0-9a-f]{40}) ([0-9]+) ([0-9]+) ([0-9]+)');
+var infoRe = new RegExp('([a-z-]+) ?(.*)');
+var curCommit = new Commit();
+
+function handleResponse() {
+	debug('handleResp ready: ' + http.readyState
+	      + ' respText null?: ' + (http.responseText === null)
+	      + ' progress: ' + inProgress);
+
+	if (http.readyState != 4 && http.readyState != 3)
+		return;
+
+	// In konqueror http.responseText is sometimes null here...
+	if (http.responseText === null)
+		return;
+
+	if (inProgress)
+		return;
+	else
+		inProgress = true;
+
+	while (prevDataLength != http.responseText.length) {
+		if (http.readyState == 4
+		    && prevDataLength == http.responseText.length) {
+			break;
+		}
+
+		prevDataLength = http.responseText.length;
+		var response = http.responseText.substring(nextLine);
+		var lines = response.split('\n');
+		nextLine = nextLine + response.lastIndexOf('\n') + 1;
+		if (response[response.length-1] != '\n') {
+			lines.pop();
+		}
+
+		for (var i = 0; i < lines.length; i++) {
+			var match = sha1Re.exec(lines[i]);
+			if (match) {
+				var sha1 = match[1];
+				var srcline = parseInt(match[2]);
+				var resline = parseInt(match[3]);
+				var numlines = parseInt(match[4]);
+				var c = commits[sha1];
+				if (!c) {
+					c = new Commit(sha1);
+					commits[sha1] = c;
+				}
+
+				c.srcline = srcline;
+				c.resline = resline;
+				c.numlines = numlines;
+				curCommit = c;
+			} else if ((match = infoRe.exec(lines[i]))) {
+				var info = match[1];
+				var data = match[2];
+				if (info == 'filename') {
+					curCommit.filename = data;
+					handleLine(curCommit);
+				} else if (info == 'author') {
+					curCommit.author = data;
+				} else if (info == 'author-time') {
+					curCommit.authorTime = parseInt(data);
+				}
+			} else if (lines[i] != '') {
+				debug('malformed line: ' + lines[i]);
+			}
+		}
+	}
+
+	if (http.readyState == 4 && prevDataLength == http.responseText.length)
+		fixColors();
+
+	inProgress = false;
+}
+
+function startBlame(blamedataUrl, bUrl)
+{
+	baseUrl = bUrl;
+	http = createRequestObject();
+	http.open('get', blamedataUrl);
+	http.onreadystatechange = handleResponse;
+	http.send(null);
+}
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 981fe41..dae35de 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -61,6 +61,8 @@ our $stylesheet = undef;
 our $logo = "++GITWEB_LOGO++";
 # URI of GIT favicon, assumed to be image/png type
 our $favicon = "++GITWEB_FAVICON++";
+# URI of blame.js
+our $blamejs = "++GITWEB_BLAMEJS++";
 
 # URI and label (title) of GIT logo link
 #our $logo_url = "http://www.kernel.org/pub/software/scm/git/docs/";
@@ -472,6 +474,8 @@ $git_dir = "$projectroot/$project" if $project;
 # dispatch
 my %actions = (
 	"blame" => \&git_blame2,
+	"blame_incremental" => \&git_blame_incremental,
+	"blame_data" => \&git_blame_data,
 	"blobdiff" => \&git_blobdiff,
 	"blobdiff_plain" => \&git_blobdiff_plain,
 	"blob" => \&git_blob,
@@ -570,7 +574,7 @@ sub href(%) {
 			push @result, $symbol . "=" . esc_param($params{$name});
 		}
 	}
-	$href .= "?" . join(';', @result) if scalar @result;
+	$href .= "?" . join(';', @result) if $params{-partial_query} or scalar @result;
 
 	return $href;
 }
@@ -1937,6 +1941,16 @@ sub git_header_html {
 <meta name="generator" content="gitweb/$version git/$git_version$mod_perl_version"/>
 <meta name="robots" content="index, nofollow"/>
 <title>$title</title>
+<script type="text/javascript">/* <![CDATA[ */
+function fixBlameLinks() {
+	var allLinks = document.getElementsByTagName("a");
+	for (var i = 0; i < allLinks.length; i++) {
+		var link = allLinks.item(i);
+		if (link.className == 'blamelink')
+			link.href = link.href.replace("a=blame", "a=blame_incremental");
+	}
+}
+/* ]]> */</script>
 EOF
 # print out each stylesheet that exist
 	if (defined $stylesheet) {
@@ -1968,7 +1982,7 @@ EOF
 	}
 
 	print "</head>\n" .
-	      "<body>\n";
+	      "<body onload=\"fixBlameLinks();\">\n";
 
 	if (-f $site_header) {
 		open (my $fd, $site_header);
@@ -2358,7 +2372,7 @@ sub git_print_tree_entry {
 		if ($have_blame) {
 			print " | " .
 			      $cgi->a({-href => href(action=>"blame", hash=>$t->{'hash'},
-			                             file_name=>"$basedir$t->{'name'}", %base_key)},
+			                             file_name=>"$basedir$t->{'name'}", %base_key), -class => "blamelink"},
 			              "blame");
 		}
 		if (defined $hash_base) {
@@ -2608,7 +2622,7 @@ sub git_difftree_body {
 			              "blob") . " | ";
 			if ($have_blame) {
 				print $cgi->a({-href => href(action=>"blame", hash_base=>$parent,
-				                             file_name=>$diff->{'file'})},
+				                             file_name=>$diff->{'file'}), -class => "blamelink"},
 				              "blame") . " | ";
 			}
 			print $cgi->a({-href => href(action=>"history", hash_base=>$parent,
@@ -2658,7 +2672,7 @@ sub git_difftree_body {
 			               "blob") . " | ";
 			if ($have_blame) {
 				print $cgi->a({-href => href(action=>"blame", hash_base=>$hash,
-				                             file_name=>$diff->{'file'})},
+				                             file_name=>$diff->{'file'}), -class => "blamelink"},
 				              "blame") . " | ";
 			}
 			print $cgi->a({-href => href(action=>"history", hash_base=>$hash,
@@ -2703,7 +2717,7 @@ sub git_difftree_body {
 			              "blob") . " | ";
 			if ($have_blame) {
 				print $cgi->a({-href => href(action=>"blame", hash_base=>$hash,
-				                             file_name=>$diff->{'to_file'})},
+				                             file_name=>$diff->{'to_file'}), -class => "blamelink"},
 				              "blame") . " | ";
 			}
 			print $cgi->a({-href => href(action=>"history", hash_base=>$hash,
@@ -3577,7 +3591,47 @@ sub git_tag {
 	git_footer_html();
 }
 
-sub git_blame2 {
+sub git_blame_data {
+	my $fd;
+	my $ftype;
+
+	my ($have_blame) = gitweb_check_feature('blame');
+	if (!$have_blame) {
+		die_error('403 Permission denied', "Permission denied");
+	}
+	die_error('404 Not Found', "File name not defined") if (!$file_name);
+	$hash_base ||= git_get_head_hash($project);
+	die_error(undef, "Couldn't find base commit") unless ($hash_base);
+	my %co = parse_commit($hash_base)
+		or die_error(undef, "Reading commit failed");
+	if (!defined $hash) {
+		$hash = git_get_hash_by_path($hash_base, $file_name, "blob")
+			or die_error(undef, "Error looking up file");
+	}
+	$ftype = git_get_type($hash);
+	if ($ftype !~ "blob") {
+		die_error("400 Bad Request", "Object is not a blob");
+	}
+	open ($fd, "-|", git_cmd(), "blame", '--incremental', $hash_base, '--',
+	      $file_name)
+		or die_error(undef, "Open git-blame --incremental failed");
+
+	print $cgi->header(-type=>"text/plain", -charset => 'utf-8',
+	                   -status=> "200 OK");
+
+	while(<$fd>) {
+ 	  if (/^([0-9a-f]{40}) ([0-9]+) ([0-9]+) ([0-9]+)/ or
+	     /^author-time |^author |^filename /) {
+ 	    print;
+	  }
+	}
+
+	close $fd or print "Reading blame data failed\n";
+}
+
+sub git_blame_common {
+	my ($type) = @_;
+
 	my $fd;
 	my $ftype;
 
@@ -3596,11 +3650,16 @@ sub git_blame2 {
 	}
 	$ftype = git_get_type($hash);
 	if ($ftype !~ "blob") {
-		die_error('400 Bad Request', "Object is not a blob");
+		die_error("400 Bad Request", "Object is not a blob");
+	}
+	if ($type eq 'incremental') {
+		open ($fd, "-|", git_cmd(), 'cat-file', 'blob', $hash)
+			or die_error(undef, "Open git-cat-file failed");
+	} else {
+		open ($fd, "-|", git_cmd(), 'blame', '-p', '--',
+		      $file_name, $hash_base)
+			or die_error(undef, "Open git-blame failed");
 	}
-	open ($fd, "-|", git_cmd(), "blame", '-p', '--',
-	      $file_name, $hash_base)
-		or die_error(undef, "Open git-blame failed");
 	git_header_html();
 	my $formats_nav =
 		$cgi->a({-href => href(action=>"blob", hash=>$hash, hash_base=>$hash_base, file_name=>$file_name)},
@@ -3609,7 +3668,7 @@ sub git_blame2 {
 		$cgi->a({-href => href(action=>"history", hash=>$hash, hash_base=>$hash_base, file_name=>$file_name)},
 			"history") .
 		" | " .
-		$cgi->a({-href => href(action=>"blame", file_name=>$file_name)},
+		$cgi->a({-href => href(action=>"blame", file_name=>$file_name), -class => "blamelink"},
 		        "HEAD");
 	git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
 	git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
@@ -3618,15 +3677,26 @@ sub git_blame2 {
 	my $num_colors = scalar(@rev_color);
 	my $current_color = 0;
 	my $last_rev;
+	print "<script type=\"text/javascript\" src=\"$blamejs\"></script>\n" if $type eq 'incremental';
 	print <<HTML;
 <div class="page_body">
 <table class="blame">
 <tr><th>Commit</th><th>Line</th><th>Data</th></tr>
 HTML
 	my %metainfo = ();
-	while (1) {
-		$_ = <$fd>;
-		last unless defined $_;
+	my $linenr = 0;
+	while (<$fd>) {
+		chomp;
+		if ($type eq 'incremental') {
+			# Empty stage with just the file contents
+			$linenr += 1;
+			print "<tr id=\"l$linenr\" class=\"light2\">";
+			print '<td class="sha1"><a href=""></a></td>';
+			print "<td class=\"linenr\"><a class=\"linenr\" href=\"\">$linenr</a></td><td class=\"pre\">" . esc_html($_) . "</td>\n";
+			print "</tr>\n";
+			next;
+		}
+
 		my ($full_rev, $orig_lineno, $lineno, $group_size) =
 		    /^([0-9a-f]{40}) (\d+) (\d+)(?: (\d+))?$/;
 		if (!exists $metainfo{$full_rev}) {
@@ -3678,13 +3748,30 @@ HTML
 		print "<td class=\"pre\">" . esc_html($data) . "</td>\n";
 		print "</tr>\n";
 	}
+
 	print "</table>\n";
 	print "</div>";
 	close $fd
 		or print "Reading blob failed\n";
+
+	if ($type eq 'incremental') {
+		print "<script type=\"text/javascript\">\n";
+		print "startBlame(\"" . href(action=>"blame_data", hash_base=>$hash_base, file_name=>$file_name) . "\", \"" .
+		  href(-partial_query=>1) . "\");\n";
+		print "</script>\n";
+	}
+
 	git_footer_html();
 }
 
+sub git_blame_incremental {
+	git_blame_common('incremental');
+}
+
+sub git_blame2 {
+	git_blame_common('oneshot');
+}
+
 sub git_blame {
 	my $fd;
 
@@ -3711,7 +3798,7 @@ sub git_blame {
 		$cgi->a({-href => href(action=>"history", hash=>$hash, hash_base=>$hash_base, file_name=>$file_name)},
 			"history") .
 		" | " .
-		$cgi->a({-href => href(action=>"blame", file_name=>$file_name)},
+		$cgi->a({-href => href(action=>"blame", file_name=>$file_name), -class => "blamelink"},
 		        "HEAD");
 	git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
 	git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
@@ -3884,7 +3971,7 @@ sub git_blob {
 			if ($have_blame) {
 				$formats_nav .=
 					$cgi->a({-href => href(action=>"blame", hash_base=>$hash_base,
-					                       hash=>$hash, file_name=>$file_name)},
+					                       hash=>$hash, file_name=>$file_name), -class => "blamelink"},
 					        "blame") .
 					" | ";
 			}
@@ -5179,7 +5266,7 @@ XML
 			              -title => "diff"}, 'D');
 			if ($have_blame) {
 				print $cgi->a({-href => href(-full=>1, action=>"blame",
-				                             file_name=>$file, hash_base=>$commit),
+				                             file_name=>$file, hash_base=>$commit), -class => "blamelink",
 				              -title => "blame"}, 'B');
 			}
 			# if this is not a feed of a file history

^ permalink raw reply related

* Re: [PATCH] git-archive: convert archive entries like checkouts do
From: René Scharfe @ 2007-05-18 22:58 UTC (permalink / raw)
  To: Daniel Barkalow, Junio C Hamano
  Cc: Johan Herland, git, Frank Lichtenheld, Thomas Glanzmann,
	Michael Gernoth
In-Reply-To: <Pine.LNX.4.64.0705181826220.18541@iabervon.org>

Daniel Barkalow schrieb:
> Conditional needs a "+ 1", too.
[...]
> Same here.

Thank you for spotting this.  Fix-up patch below.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>

---
Embarrassing.  I'm off to go to sleep now.

 archive-tar.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/archive-tar.c b/archive-tar.c
index eb0abc7..33e7657 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -82,7 +82,7 @@ static void strbuf_append_string(struct strbuf *sb, const char *s)
 {
 	int slen = strlen(s);
 	int total = sb->len + slen;
-	if (total > sb->alloc) {
+	if (total + 1 > sb->alloc) {
 		sb->buf = xrealloc(sb->buf, total + 1);
 		sb->alloc = total + 1;
 	}
@@ -271,7 +271,7 @@ static int write_tar_entry(const unsigned char *sha1,
 		path.alloc = PATH_MAX;
 		path.len = path.eof = 0;
 	}
-	if (path.alloc < baselen + filenamelen) {
+	if (path.alloc < baselen + filenamelen + 1) {
 		free(path.buf);
 		path.buf = xmalloc(baselen + filenamelen + 1);
 		path.alloc = baselen + filenamelen + 1;

^ permalink raw reply related

* Re: [PATCH 12/16] builtin-fetch--tool: extend "native-store" for use in cloning
From: Alex Riesen @ 2007-05-18 22:52 UTC (permalink / raw)
  To: skimo; +Cc: git, Junio C Hamano
In-Reply-To: <11795163072266-git-send-email-skimo@liacs.nl>

skimo@liacs.nl, Fri, May 18, 2007 21:25:01 +0200:
> @@ -261,7 +287,8 @@ static int fetch_native_store(FILE *fp,
>  			      const char *remote,
>  			      const char *remote_nick,
>  			      const char *refs,
> -			      int verbose, int force)
> +			      int verbose, int force,
> +			      int all, int use_separate_remote)
>  {
>  	char buffer[1024];
>  	int err = 0;
> @@ -294,8 +321,12 @@ static int fetch_native_store(FILE *fp,
>  			continue;
>  		}
>  
> -		local_name = find_local_name(cp, refs,
> -					     &single_force, &not_for_merge);
> +		if (all)
> +			local_name = construct_local_name(cp, remote_nick,
> +							  use_separate_remote);
> +		else
> +			local_name = find_local_name(cp, refs,
> +						     &single_force, &not_for_merge);

This code produces warning about possible uninitialized used of
single_force and not_for_merge. I used the patch below, but didn't
look into what the "all" does.

---
 builtin-fetch--tool.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
index 12adb38..ebb49d9 100644
--- a/builtin-fetch--tool.c
+++ b/builtin-fetch--tool.c
@@ -273,7 +273,7 @@ static int fetch_native_store(FILE *fp,
 		int len;
 		char *cp;
 		char *local_name;
-		int single_force, not_for_merge;
+		int single_force = force, not_for_merge = 0;
 
 		for (cp = buffer; *cp && !isspace(*cp); cp++)
 			;
@@ -301,7 +301,7 @@ static int fetch_native_store(FILE *fp,
 		err |= append_fetch_head(fp,
 					 buffer, remote, cp, remote_nick,
 					 local_name, not_for_merge,
-					 verbose, force || single_force);
+					 verbose, single_force);
 	}
 	return err;
 }
-- 
1.5.2.rc3.83.gbbb0

^ permalink raw reply related

* [PATCH] Use run_command_v_opt_cd when checking out a submodule
From: Alex Riesen @ 2007-05-18 22:48 UTC (permalink / raw)
  To: skimo; +Cc: git, Junio C Hamano
In-Reply-To: <20070518222015.GE10475@steel.home>

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 entry.c |   12 +-----------
 1 files changed, 1 insertions(+), 11 deletions(-)

diff --git a/entry.c b/entry.c
index 96a4a60..0316c74 100644
--- a/entry.c
+++ b/entry.c
@@ -166,7 +166,6 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout
 
 static int checkout_submodule(const char *path, struct cache_entry *ce, const struct checkout *state)
 {
-	static char cwd[PATH_MAX];
 	const char *gitdirenv;
 	const char *args[10];
 	int argc;
@@ -175,12 +174,6 @@ static int checkout_submodule(const char *path, struct cache_entry *ce, const st
 	if (!state->submodules)
 		return 0;
 
-	if (!getcwd(cwd, sizeof(cwd)) || cwd[0] != '/')
-		die("Unable to read current working directory");
-
-	if (chdir(path))
-		die("Cannot move to '%s'", path);
-
 	argc = 0;
 	args[argc++] = "checkout";
 	if (state->force)
@@ -190,12 +183,9 @@ static int checkout_submodule(const char *path, struct cache_entry *ce, const st
 
 	gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
 	unsetenv(GIT_DIR_ENVIRONMENT);
-	err = run_command_v_opt(args, RUN_GIT_CMD);
+	err = run_command_v_opt_cd(args, RUN_GIT_CMD, path);
 	setenv(GIT_DIR_ENVIRONMENT, gitdirenv, 1);
 
-	if (chdir(cwd))
-		die("Cannot come back to cwd");
-
 	if (err)
 		return error("failed to run git-checkout in submodule '%s'", path);
 
-- 
1.5.2.rc3.83.gbbb0

^ permalink raw reply related

* Re: [PATCH 07/16] git-read-tree: take --submodules option
From: Alex Riesen @ 2007-05-18 22:42 UTC (permalink / raw)
  To: skimo; +Cc: git, Junio C Hamano
In-Reply-To: <20070518220826.GM942MdfPADPa@greensroom.kotnet.org>

Sven Verdoolaege, Sat, May 19, 2007 00:08:26 +0200:
> I noticed there's a whole thread about subprojects that I haven't read yet,
> so this may have been addressed already ....

Not the checkout, which is strange. It's mostly about cloning.

> On Fri, May 18, 2007 at 11:53:12PM +0200, Alex Riesen wrote:
> > Can we have this option (and corresponding support in the following
> > patches, of course) first?
> 
> That's why the clone thing comes last.
> 
> > It is enough to have subprojects working
> > locally, and people can start using them immediately: anyone can clone
> > the subprojects manually if he wishes so.
> 
> Anyone can run git-write-tree and git-commit-tree is she wishes so...

It is much more tedious. It have to be done recursively, and with
right SHA and you have to cd into right direcotry first and it is
git-read-tree and git-checkout-index, BTW.

IOW, it is hard.

> The reason for not putting this in shouldn't be that someone doesn't
> think it is useful; the reason should be that my code is crap.

The code is not a problem. It can be also discarded because you
implemented something no one wants.

I just meant to say, that even if no one wants your subproject cloning
code, _I_ support your checkout effort and I am asking for it to be
put in.

"First", as the cloning discussion does not seem to be finished (and,
as I said, I am not interested in cloning anyway).

^ permalink raw reply

* Re: [PATCH 09/16] entry.c: optionally checkout submodules
From: Alex Riesen @ 2007-05-18 22:33 UTC (permalink / raw)
  To: skimo; +Cc: git, Junio C Hamano
In-Reply-To: <20070518220323.GL942MdfPADPa@greensroom.kotnet.org>

Sven Verdoolaege, Sat, May 19, 2007 00:03:23 +0200:
> On Fri, May 18, 2007 at 11:56:42PM +0200, Alex Riesen wrote:
> > skimo@liacs.nl, Fri, May 18, 2007 21:24:58 +0200:
> > > +	if (err)
> > > +		return error("failed to run git-checkout in submodule '%s'", path);
> > 
> > We may need an option to ignore these failures. Maybe even active by
> > default. Imagine a superproject with _optional_ submodules, where it
> > is just nice to know that some submodules weren't checked out. BTW,
> > doesn't git-checkout already prints an error?
> 
> Probably.  You probably noticed that I haven't written any tests yet...
> 

I see. It was a very ... provocative patch series :)

> Still, the error that git-checkout prints may not give enough of a clue
> that something was wrong with a submodule.

Like, for example, it failed because the directory is not a git repo
yet, because the previous git-checkout was called _without_
--submodule and the directories created are just empty.

Anyway, just a "failed to run git-checkout" is not very helpful
either. Come to think about it, there is not very much you can tell
out of super-project context. git-checkout will always know better.

^ permalink raw reply

* Re: [PATCH 05/16] unpack-trees.c: verify_uptodate: remove dead code
From: Junio C Hamano @ 2007-05-18 22:33 UTC (permalink / raw)
  To: skimo; +Cc: git
In-Reply-To: <11795163061911-git-send-email-skimo@liacs.nl>

skimo@liacs.nl writes:

> From: Sven Verdoolaege <skimo@kotnet.org>
>
> This code was killed by commit fcc387db9bc453dc7e07a262873481af2ee9e5c8.
>
> Signed-off-by: Sven Verdoolaege <skimo@kotnet.org>
> ---
>  unpack-trees.c |    4 ----
>  1 files changed, 0 insertions(+), 4 deletions(-)
>
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 906ce69..cac2411 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -414,10 +414,6 @@ static void verify_uptodate(struct cache_entry *ce,
>  			return;
>  		errno = 0;
>  	}
> -	if (o->reset) {
> -		ce->ce_flags |= htons(CE_UPDATE);
> -		return;
> -	}
>  	if (errno == ENOENT)
>  		return;
>  	die("Entry '%s' not uptodate. Cannot merge.", ce->name);
> -- 
> 1.5.2.rc3.783.gc7476-dirty

Hmmm.

I am not absolutely sure if the fcc387db change was correct
anymore, but in any case, this removal of dead code should not
break anything.

But this does not belong to your series either.

Perhaps I should apply this to 'master' regardless of the rest
of the series.

^ permalink raw reply

* Re: [PATCH 11/16] git-fetch: skip empty arguments
From: Junio C Hamano @ 2007-05-18 22:33 UTC (permalink / raw)
  To: skimo; +Cc: git
In-Reply-To: <117951630747-git-send-email-skimo@liacs.nl>

skimo@liacs.nl writes:

> From: Sven Verdoolaege <skimo@kotnet.org>
>
> This makes it easier for scripts to call git-fetch with options
> that may or may not be set.

For git-fetch it does not matter as I do not think there is any
valid case to pass an empty string as a parameter to it (even
"fetch from our own repository" requires a single dot).  But
from discipline point of view, I am not happy about this.

If you are talking about shell scripts, the standard way to do
that is to say ${1+"$1"}.

^ permalink raw reply

* Re: [PATCH] git-archive: convert archive entries like checkouts do
From: Daniel Barkalow @ 2007-05-18 22:27 UTC (permalink / raw)
  To: René Scharfe
  Cc: Junio C Hamano, Johan Herland, git, Frank Lichtenheld,
	Thomas Glanzmann, Michael Gernoth
In-Reply-To: <464E2425.2030904@lsrfire.ath.cx>

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

On Sat, 19 May 2007, René Scharfe wrote:

> diff --git a/archive-tar.c b/archive-tar.c
> index 56ff356..eb0abc7 100644
> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -83,11 +83,12 @@ static void strbuf_append_string(struct strbuf *sb, const char *s)
>  	int slen = strlen(s);
>  	int total = sb->len + slen;
>  	if (total > sb->alloc) {
> -		sb->buf = xrealloc(sb->buf, total);
> -		sb->alloc = total;
> +		sb->buf = xrealloc(sb->buf, total + 1);
> +		sb->alloc = total + 1;

Conditional needs a "+ 1", too.

>  	}
>  	memcpy(sb->buf + sb->len, s, slen);
>  	sb->len = total;
> +	sb->buf[total] = '\0';
>  }
>  
>  /*
> @@ -272,18 +273,19 @@ static int write_tar_entry(const unsigned char *sha1,
>  	}
>  	if (path.alloc < baselen + filenamelen) {
>  		free(path.buf);
> -		path.buf = xmalloc(baselen + filenamelen);
> -		path.alloc = baselen + filenamelen;
> +		path.buf = xmalloc(baselen + filenamelen + 1);
> +		path.alloc = baselen + filenamelen + 1;

Same here.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* [PATCH] Add run_command_v_opt_cd: chdir into a directory before exec
From: Alex Riesen @ 2007-05-18 22:20 UTC (permalink / raw)
  To: skimo; +Cc: git, Junio C Hamano
In-Reply-To: <20070518220014.GD10475@steel.home>

It can make code simplier (no need to preserve cwd) and safer
(no chance the cwd of the current process is accidentally forgotten).

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
Alex Riesen, Sat, May 19, 2007 00:00:14 +0200:
> skimo@liacs.nl, Fri, May 18, 2007 21:24:58 +0200:
> > +	if (!getcwd(cwd, sizeof(cwd)) || cwd[0] != '/')
> > +		die("Unable to read current working directory");
> > +
> > +	if (chdir(path))
> > +		die("Cannot move to '%s'", path);
> > +
> 
> How about modifying run_command to chdir after fork?
> 
> You'd save the hassle of save/restoring cwd and don't mess up process'
> context (which is always a good idea to preserve). The code'd be
> simplier, too.
> 

something like this

 run-command.c |   27 ++++++++++++++++++++++-----
 run-command.h |    2 ++
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/run-command.c b/run-command.c
index eff523e..043b570 100644
--- a/run-command.c
+++ b/run-command.c
@@ -73,6 +73,9 @@ int start_command(struct child_process *cmd)
 			close(cmd->out);
 		}
 
+		if (cmd->dir && chdir(cmd->dir))
+			die("exec %s: cd to %s failed (%s)", cmd->argv[0],
+			    cmd->dir, strerror(errno));
 		if (cmd->git_cmd) {
 			execv_git_cmd(cmd->argv);
 		} else {
@@ -133,13 +136,27 @@ int run_command(struct child_process *cmd)
 	return finish_command(cmd);
 }
 
+static void prepare_run_command_v_opt(struct child_process *cmd,
+				      const char **argv, int opt)
+{
+	memset(cmd, 0, sizeof(*cmd));
+	cmd->argv = argv;
+	cmd->no_stdin = opt & RUN_COMMAND_NO_STDIN ? 1 : 0;
+	cmd->git_cmd = opt & RUN_GIT_CMD ? 1 : 0;
+	cmd->stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0;
+}
+
 int run_command_v_opt(const char **argv, int opt)
 {
 	struct child_process cmd;
-	memset(&cmd, 0, sizeof(cmd));
-	cmd.argv = argv;
-	cmd.no_stdin = opt & RUN_COMMAND_NO_STDIN ? 1 : 0;
-	cmd.git_cmd = opt & RUN_GIT_CMD ? 1 : 0;
-	cmd.stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0;
+	prepare_run_command_v_opt(&cmd, argv, opt);
+	return run_command(&cmd);
+}
+
+int run_command_v_opt_cd(const char **argv, int opt, const char *dir)
+{
+	struct child_process cmd;
+	prepare_run_command_v_opt(&cmd, argv, opt);
+	cmd.dir = dir;
 	return run_command(&cmd);
 }
diff --git a/run-command.h b/run-command.h
index 3680ef9..cbd7484 100644
--- a/run-command.h
+++ b/run-command.h
@@ -16,6 +16,7 @@ struct child_process {
 	pid_t pid;
 	int in;
 	int out;
+	const char *dir;
 	unsigned close_in:1;
 	unsigned close_out:1;
 	unsigned no_stdin:1;
@@ -32,5 +33,6 @@ int run_command(struct child_process *);
 #define RUN_GIT_CMD	     2	/*If this is to be git sub-command */
 #define RUN_COMMAND_STDOUT_TO_STDERR 4
 int run_command_v_opt(const char **argv, int opt);
+int run_command_v_opt_cd(const char **argv, int opt, const char *dir);
 
 #endif
-- 
1.5.2.rc3.83.gbbb0

^ permalink raw reply related

* Re: [PATCH] gitweb: Extra columns in blame
From: Petr Baudis @ 2007-05-18 22:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7jl4zjm.fsf@assigned-by-dhcp.cox.net>

On Fri, May 18, 2007 at 11:01:01PM CEST, Junio C Hamano wrote:
> Good job, except that I think you should also show the filename
> especially as you seem to run with -C (I haven't looked at the
> code yet, though).
> 
> E.g.
> 
> http://repo.or.cz/w/linux-2.6.git?a=blame_incremental;f=block/ll_rw_blk.c;h=6b5173ac81313d8adb5c1d7b521559f565bb209b;hb=347b4599dd6ffef27e18c227532d1ec66556000b
> 
> the first few hunks that came from 1da177e4 are from a different
> file, drivers/block/ll_rw_blk.c.

I don't use git-blame -C - I pass virtually no extra parameters to
git-blame (except some output controlling). Passing -C to git-blame
might be an interesting idea but the possible performance hit is a bit
scary; when the dust settles and this gets merged or something, I can
experiment with it a bit further...

(Also, I fear a bit about making it _too_ wide even with the extra
columns; there should be reasonable portion of line still visible on
usual resolutions with usual font sizes. Might be nice UI challenge.

> Also the incremental thing using JavaScript does not seem to
> work for me incrementally for some reason, although if I wait
> long enough I get the fully blamed picture that seems to match
> nonincremental one.  While I am waiting, the browser goes silent
> and does not even let me switch to other tabs, so it is not all
> that useful to me in its current shape.

Strange, what browser are you using?

The trouble is, I'm not really very good at this kind of web development
because I have access only to a rather narrow portion of the browser
market - Firefox at Linux, at work also Galeon, Epiphany and Konqueror,
and of course ELinks. I have theoretical access to MSIE at work but it's
quite a hassle. With all browser I've tested it with, it worked without
a problem, so I'm not sure how much will I be able to debug it (and I'm
really bad at debugging javascript anyway).

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Ever try. Ever fail. No matter. // Try again. Fail again. Fail better.
		-- Samuel Beckett

^ permalink raw reply

* [PATCH] git-archive: convert archive entries like checkouts do
From: René Scharfe @ 2007-05-18 22:09 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johan Herland, git, Frank Lichtenheld, Thomas Glanzmann,
	Michael Gernoth
In-Reply-To: <200705171928.34927.johan@herland.net>

As noted by Johan Herland, git-archive is a kind of checkout and needs
to apply any checkout filters that might be configured.

This patch adds the convenience function convert_sha1_file which returns
a buffer containing the object's contents, after converting, if necessary
(i.e. it's a combination of read_sha1_file and convert_to_working_tree).
Direct calls to read_sha1_file in git-archive are then replaced by calls
to convert_sha1_file.

Since convert_sha1_file expects its path argument to be NUL-terminated --
a convention it inherits from convert_to_working_tree -- the patch also
changes the path handling in archive-tar.c to always NUL-terminate the
string.  It used to solely rely on the len field of struct strbuf before.

archive-zip.c already NUL-terminates the path and thus needs no such
change.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>

---
 archive-tar.c |   12 +++++++-----
 archive-zip.c |    2 +-
 cache.h       |    1 +
 convert.c     |   15 +++++++++++++++
 4 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/archive-tar.c b/archive-tar.c
index 56ff356..eb0abc7 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -83,11 +83,12 @@ static void strbuf_append_string(struct strbuf *sb, const char *s)
 	int slen = strlen(s);
 	int total = sb->len + slen;
 	if (total > sb->alloc) {
-		sb->buf = xrealloc(sb->buf, total);
-		sb->alloc = total;
+		sb->buf = xrealloc(sb->buf, total + 1);
+		sb->alloc = total + 1;
 	}
 	memcpy(sb->buf + sb->len, s, slen);
 	sb->len = total;
+	sb->buf[total] = '\0';
 }
 
 /*
@@ -272,18 +273,19 @@ static int write_tar_entry(const unsigned char *sha1,
 	}
 	if (path.alloc < baselen + filenamelen) {
 		free(path.buf);
-		path.buf = xmalloc(baselen + filenamelen);
-		path.alloc = baselen + filenamelen;
+		path.buf = xmalloc(baselen + filenamelen + 1);
+		path.alloc = baselen + filenamelen + 1;
 	}
 	memcpy(path.buf, base, baselen);
 	memcpy(path.buf + baselen, filename, filenamelen);
 	path.len = baselen + filenamelen;
+	path.buf[path.len] = '\0';
 	if (S_ISDIR(mode) || S_ISDIRLNK(mode)) {
 		strbuf_append_string(&path, "/");
 		buffer = NULL;
 		size = 0;
 	} else {
-		buffer = read_sha1_file(sha1, &type, &size);
+		buffer = convert_sha1_file(path.buf, sha1, mode, &type, &size);
 		if (!buffer)
 			die("cannot read %s", sha1_to_hex(sha1));
 	}
diff --git a/archive-zip.c b/archive-zip.c
index 1eaf262..3cbf6bb 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -195,7 +195,7 @@ static int write_zip_entry(const unsigned char *sha1,
 		if (S_ISREG(mode) && zlib_compression_level != 0)
 			method = 8;
 		result = 0;
-		buffer = read_sha1_file(sha1, &type, &size);
+		buffer = convert_sha1_file(path, sha1, mode, &type, &size);
 		if (!buffer)
 			die("cannot read %s", sha1_to_hex(sha1));
 		crc = crc32(crc, buffer, size);
diff --git a/cache.h b/cache.h
index aaeb04a..4204bc1 100644
--- a/cache.h
+++ b/cache.h
@@ -548,6 +548,7 @@ extern void trace_argv_printf(const char **argv, int count, const char *format,
 /* convert.c */
 extern char *convert_to_git(const char *path, const char *src, unsigned long *sizep);
 extern char *convert_to_working_tree(const char *path, const char *src, unsigned long *sizep);
+extern void *convert_sha1_file(const char *path, const unsigned char *sha1, unsigned int mode, enum object_type *type, unsigned long *size);
 
 /* match-trees.c */
 void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, int);
diff --git a/convert.c b/convert.c
index 12abdaf..c64880b 100644
--- a/convert.c
+++ b/convert.c
@@ -652,3 +652,18 @@ char *convert_to_working_tree(const char *path, const char *src, unsigned long *
 
 	return buf;
 }
+
+void *convert_sha1_file(const char *path, const unsigned char *sha1,
+                        unsigned int mode, enum object_type *type,
+                        unsigned long *size)
+{
+	void *buffer = read_sha1_file(sha1, type, size);
+	if (S_ISREG(mode) && buffer) {
+		void *converted = convert_to_working_tree(path, buffer, size);
+		if (converted) {
+			free(buffer);
+			buffer = converted;
+		}
+	}
+	return buffer;
+}

^ permalink raw reply related

* Re: [PATCH 07/16] git-read-tree: take --submodules option
From: Sven Verdoolaege @ 2007-05-18 22:08 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git, Junio C Hamano
In-Reply-To: <20070518215312.GB10475@steel.home>

I noticed there's a whole thread about subprojects that I haven't read yet,
so this may have been addressed already ....

On Fri, May 18, 2007 at 11:53:12PM +0200, Alex Riesen wrote:
> Can we have this option (and corresponding support in the following
> patches, of course) first?

That's why the clone thing comes last.

> It is enough to have subprojects working
> locally, and people can start using them immediately: anyone can clone
> the subprojects manually if he wishes so.

Anyone can run git-write-tree and git-commit-tree is she wishes so...

> Cloning of subprojects is still unclear, and frankly I'm not sure it
> should be done at all. Not even with an option which is off by
> default.

Then don't use it.

The reason for not putting this in shouldn't be that someone doesn't
think it is useful; the reason should be that my code is crap.

skimo

^ permalink raw reply

* Re: [PATCH 09/16] entry.c: optionally checkout submodules
From: Sven Verdoolaege @ 2007-05-18 22:03 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git, Junio C Hamano
In-Reply-To: <20070518215642.GC10475@steel.home>

On Fri, May 18, 2007 at 11:56:42PM +0200, Alex Riesen wrote:
> skimo@liacs.nl, Fri, May 18, 2007 21:24:58 +0200:
> > +	if (err)
> > +		return error("failed to run git-checkout in submodule '%s'", path);
> 
> We may need an option to ignore these failures. Maybe even active by
> default. Imagine a superproject with _optional_ submodules, where it
> is just nice to know that some submodules weren't checked out. BTW,
> doesn't git-checkout already prints an error?

Probably.  You probably noticed that I haven't written any tests yet...

Still, the error that git-checkout prints may not give enough of a clue
that something was wrong with a submodule.

skimo

^ permalink raw reply

* Re: [PATCH 09/16] entry.c: optionally checkout submodules
From: Alex Riesen @ 2007-05-18 22:00 UTC (permalink / raw)
  To: skimo; +Cc: git, Junio C Hamano
In-Reply-To: <1179516307425-git-send-email-skimo@liacs.nl>

skimo@liacs.nl, Fri, May 18, 2007 21:24:58 +0200:
> +	if (!getcwd(cwd, sizeof(cwd)) || cwd[0] != '/')
> +		die("Unable to read current working directory");
> +
> +	if (chdir(path))
> +		die("Cannot move to '%s'", path);
> +

How about modifying run_command to chdir after fork?

You'd save the hassle of save/restoring cwd and don't mess up process'
context (which is always a good idea to preserve). The code'd be
simplier, too.

^ permalink raw reply

* Re: Smart fetch via HTTP?
From: Joel Becker @ 2007-05-18 21:56 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Matthieu Moy, git
In-Reply-To: <alpine.LFD.0.98.0705181312060.3890@woody.linux-foundation.org>

On Fri, May 18, 2007 at 01:13:36PM -0700, Linus Torvalds wrote:
> If it's _just_ the initial GET/CONNECT strings, yeah, we could probably 
> easily make the git-daemon just ignore them. That shouldn't be a problem.
> 
> But if there's anything *else* required, it gets uglier much more quickly.

	With CONNECT, there isn't anything.  That is, your
GIT_PROXY_COMMAND handles talking to the proxy, then gives git itself a
raw data pipe.  My proxy allows CONNECT to 9418, and that's how I use it
today.
	If you tried to make POST work (It'd be POST, not GET, as you
need to connect up the sending side), either apache would have to front
it for us, or "git-daemon --http" would have to accept the HTTP headers
on before the input, and output a proper HTTP response before sending
output.  Seeing the headers would allow for us to vhost, even.
	Hmm, but the proxy may not allow two-way communication.  Does
the git protocol have more than one round-trip?  That is:

Client:
    POST http://server.git.host:80/projects/thisproject HTTP/1.1
    Host: server.git.host

    fetch-pack <sha1>
    EOF

Server:
    200 OK HTTP/1.1
    
    <data>
    EOF

should work, I'd think.

Joel


-- 

"Ninety feet between bases is perhaps as close as man has ever come
 to perfection."
	- Red Smith

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

^ permalink raw reply

* Re: [PATCH 09/16] entry.c: optionally checkout submodules
From: Alex Riesen @ 2007-05-18 21:56 UTC (permalink / raw)
  To: skimo; +Cc: git, Junio C Hamano
In-Reply-To: <1179516307425-git-send-email-skimo@liacs.nl>

skimo@liacs.nl, Fri, May 18, 2007 21:24:58 +0200:
> +	if (err)
> +		return error("failed to run git-checkout in submodule '%s'", path);

We may need an option to ignore these failures. Maybe even active by
default. Imagine a superproject with _optional_ submodules, where it
is just nice to know that some submodules weren't checked out. BTW,
doesn't git-checkout already prints an error?

^ permalink raw reply

* Re: merge summaries
From: J. Bruce Fields @ 2007-05-18 21:56 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, Steffen Prohaska, Git Mailing List
In-Reply-To: <alpine.LFD.0.98.0705181206051.3890@woody.linux-foundation.org>

On Fri, May 18, 2007 at 12:13:02PM -0700, Linus Torvalds wrote:
> 
> On Fri, 18 May 2007, Junio C Hamano wrote:
> > 
> > This does not necessarily mean that your lieutenants should not
> > use merge.summary when they pull from other trees (or inside
> > their own repository to merge in the topics).  They need to
> > however disable it with --no-summary when they pull from you
> > when they choose to merge instead of rebase to adjust to the
> > updated infrastructure in your tree.
> 
> Yes, the problem is that people *will* get it wrong, so right now I'd 
> discourage people from even trying to enable merge summaries unless they 
> are the top-level maintainer.

I never quite understood what they're for--do they add any information
not already available in the history?  If not, and if people still find
them helpful anyway, then I dunno, it looks like a sign of some sort of
failure of our history display tools.

--b.

^ permalink raw reply

* Re: [PATCH 07/16] git-read-tree: take --submodules option
From: Alex Riesen @ 2007-05-18 21:53 UTC (permalink / raw)
  To: skimo; +Cc: git, Junio C Hamano
In-Reply-To: <11795163061588-git-send-email-skimo@liacs.nl>

skimo@liacs.nl, Fri, May 18, 2007 21:24:56 +0200:
> 
> This option currently has no effect.
> 

Can we have this option (and corresponding support in the following
patches, of course) first? It is enough to have subprojects working
locally, and people can start using them immediately: anyone can clone
the subprojects manually if he wishes so.

Cloning of subprojects is still unclear, and frankly I'm not sure it
should be done at all. Not even with an option which is off by
default.

^ permalink raw reply

* Re: [PATCH] gitweb: Extra columns in blame
From: Junio C Hamano @ 2007-05-18 21:01 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20070518191725.10460.83338.stgit@rover>

Good job, except that I think you should also show the filename
especially as you seem to run with -C (I haven't looked at the
code yet, though).

E.g.

http://repo.or.cz/w/linux-2.6.git?a=blame_incremental;f=block/ll_rw_blk.c;h=6b5173ac81313d8adb5c1d7b521559f565bb209b;hb=347b4599dd6ffef27e18c227532d1ec66556000b

the first few hunks that came from 1da177e4 are from a different
file, drivers/block/ll_rw_blk.c.

Also the incremental thing using JavaScript does not seem to
work for me incrementally for some reason, although if I wait
long enough I get the fully blamed picture that seems to match
nonincremental one.  While I am waiting, the browser goes silent
and does not even let me switch to other tabs, so it is not all
that useful to me in its current shape.

^ permalink raw reply

* Re: Smart fetch via HTTP?
From: Linus Torvalds @ 2007-05-18 20:13 UTC (permalink / raw)
  To: Joel Becker; +Cc: Matthieu Moy, git
In-Reply-To: <20070518190159.GS24644@ca-server1.us.oracle.com>



On Fri, 18 May 2007, Joel Becker wrote:
> 
> 	It's not about packet scanning, it's about GET vs CONNECT.  If
> the proxy allows GET but not CONNECT, it's going to forward the HTTP
> protocol to the server, and git-daemon is going to see "GET /project
> HTTP/1.1" as its first input.  Now, perhaps we can cook that up behind
> some apache so that apache handles vhosting the URL, then calls
> git-daemon which can take the stdin.  So we'd be doing POST, not GET.

If it's _just_ the initial GET/CONNECT strings, yeah, we could probably 
easily make the git-daemon just ignore them. That shouldn't be a problem.

But if there's anything *else* required, it gets uglier much more quickly.

		Linus

^ permalink raw reply

* Re: Smart fetch via HTTP?
From: Matthieu Moy @ 2007-05-18 20:06 UTC (permalink / raw)
  To: Joel Becker; +Cc: Linus Torvalds, git
In-Reply-To: <20070518190159.GS24644@ca-server1.us.oracle.com>

Joel Becker <Joel.Becker@oracle.com> writes:

> 	A normal company needs to have their firewall allow CONNECT to
> 9418.  Then git proxying over HTTP is possible to a standard
> git-daemon.

443 should work too (that's HTTPS, and the proxy can't filter it,
since this would be a man-in-the-middle attack).

-- 
Matthieu

^ 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