Git development
 help / color / mirror / Atom feed
* Re: [PATCH/RFC] gitweb: add the ability to show side-by-side diff on commitdiff.
From: Kato Kazuyoshi @ 2011-10-17  1:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vobxgpixo.fsf@alter.siamese.dyndns.org>

On Mon, Oct 17, 2011 at 9:20 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> What are these changes for, except perhaps to make the patch larger than
> necesssary to make it harder to review?
>
> It would leave an unnecessary SP like 'class="diff "' when all the arms of
> if/elsif cascade fall off and $diff_class is left empty. It isn't a major
> issue (I suspect that such a case might even be an error), and I even
> think the code after the above patch would be easier to read and more
> sensible, but shouldn't that kind of "style fix" be in a separate clean-up
> patch that does not introduce any new feature?
>

Sorry. I will separate my patch into two or three commits. Probably like:

- format_diff_line returns a class with an line.
- remove trailing space from the class.
- add side-by-side feature and CSS.
- add form.

Thank you for your correction!

-- 
Kato Kazuyoshi

^ permalink raw reply

* Re: regression in git-gui since 2c5c66b... Merge branch 'jp/get-ref-dir-unsorted
From: Junio C Hamano @ 2011-10-17  0:35 UTC (permalink / raw)
  To: Mark Levedahl; +Cc: git
In-Reply-To: <4E9B1E32.7030101@gmail.com>

Mark Levedahl <mlevedahl@gmail.com> writes:

> I have a project organized as a number of nested git modules (not
> using git-submodule), and frequently use git-new-workdir to create the
> nested modules.
>
> Since the above merge-commit, git-gui is confused by this arrangement
> and reports every file in every nested module as being an untracked
> file in the top-level (super) project.

Could you come up with a simple reproduction recipe that prepares a
superproject that has no file of its own, a submodule with a single file
tracked, and attaches another workdir? If running git-gui in the resulting
directory makes it misbehave, we could then isolate what git command that
is invoked by git-gui has changed its behaviour.

Thanks.

^ permalink raw reply

* Re: [PATCH/RFC] gitweb: add the ability to show side-by-side diff on commitdiff.
From: Junio C Hamano @ 2011-10-17  0:20 UTC (permalink / raw)
  To: Kato Kazuyoshi; +Cc: git
In-Reply-To: <CAFo4x0Kb651CyxoP8wxR36aDe5=Md2UV3qjh+HPo4ad6NB=Emg@mail.gmail.com>

Kato Kazuyoshi <kato.kazuyoshi@gmail.com> writes:

> @@ -2235,25 +2238,25 @@ sub format_diff_line {
>  		# combined diff
>  		my $prefix = substr($line, 0, scalar @{$from->{'href'}});
>  		if ($line =~ m/^\@{3}/) {
> -			$diff_class = " chunk_header";
> +			$diff_class = "chunk_header";
>  		} elsif ($line =~ m/^\\/) {
> -			$diff_class = " incomplete";
> +			$diff_class = "incomplete";
>  		} elsif ($prefix =~ tr/+/+/) {
> -			$diff_class = " add";
> +			$diff_class = "add";
>  		} elsif ($prefix =~ tr/-/-/) {
> -			$diff_class = " rem";
> +			$diff_class = "rem";
>  		}
>  	} else {
>  		# assume ordinary diff
>  		my $char = substr($line, 0, 1);
>  		if ($char eq '+') {
> -			$diff_class = " add";
> +			$diff_class = "add";
>  		} elsif ($char eq '-') {
> -			$diff_class = " rem";
> +			$diff_class = "rem";
>  		} elsif ($char eq '@') {
> -			$diff_class = " chunk_header";
> +			$diff_class = "chunk_header";
>  		} elsif ($char eq "\\") {
> -			$diff_class = " incomplete";
> +			$diff_class = "incomplete";
>  		}
>  	}
>  	$line = untabify($line);
> @@ -2274,7 +2277,7 @@ sub format_diff_line {
>  		}
>  		$line = "<span class=\"chunk_info\">@@ $from_text $to_text @@</span>" .
>  		        "<span class=\"section\">" . esc_html($section, -nbsp=>1) .
> "</span>";
> -		return "<div class=\"diff$diff_class\">$line</div>\n";
> +		return $diff_class, "<div class=\"diff $diff_class\">$line</div>\n";

What are these changes for, except perhaps to make the patch larger than
necesssary to make it harder to review?

It would leave an unnecessary SP like 'class="diff "' when all the arms of
if/elsif cascade fall off and $diff_class is left empty. It isn't a major
issue (I suspect that such a case might even be an error), and I even
think the code after the above patch would be easier to read and more
sensible, but shouldn't that kind of "style fix" be in a separate clean-up
patch that does not introduce any new feature?

> @@ -2307,9 +2310,9 @@ sub format_diff_line {
>  		}
>  		$line .= " $prefix</span>" .
>  		         "<span class=\"section\">" . esc_html($section, -nbsp=>1)
> . "</span>";
> -		return "<div class=\"diff$diff_class\">$line</div>\n";
> +		return $diff_class, "<div class=\"diff $diff_class\">$line</div>\n";
>  	}
> -	return "<div class=\"diff$diff_class\">" . esc_html($line, -nbsp=>1)
> . "</div>\n";
> +	return $diff_class, "<div class=\"diff $diff_class\">" .
> esc_html($line, -nbsp=>1) . "</div>\n";
>  }

And everything else, including this hunk to change what is returned from
the subroutine belongs to a separate patch that implements the side-by-side
feature.

^ permalink raw reply

* [PATCH/RFC] gitweb: add the ability to show side-by-side diff on commitdiff.
From: Kato Kazuyoshi @ 2011-10-16 23:57 UTC (permalink / raw)
  To: git

---
Hello,

I want to add some features that provided by Trac to Gitweb.
First, I've implemented "side-by-side" style diff.

 gitweb/gitweb.perl       |   97 ++++++++++++++++++++++++++++++++++++++--------
 gitweb/static/gitweb.css |   16 ++++++++
 2 files changed, 96 insertions(+), 17 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 85d64b2..9d7609f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -757,6 +757,7 @@ our @cgi_param_mapping = (
 	extra_options => "opt",
 	search_use_regexp => "sr",
 	ctag => "by_tag",
+	diff_style => "ds",
 	# this must be last entry (for manipulation from JavaScript)
 	javascript => "js"
 );
@@ -1072,6 +1073,8 @@ sub evaluate_and_validate_params {
 		}
 		$search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext;
 	}
+
+	$input_params{diff_style} ||= 'inline';
 }

 # path to the current git repository
@@ -2235,25 +2238,25 @@ sub format_diff_line {
 		# combined diff
 		my $prefix = substr($line, 0, scalar @{$from->{'href'}});
 		if ($line =~ m/^\@{3}/) {
-			$diff_class = " chunk_header";
+			$diff_class = "chunk_header";
 		} elsif ($line =~ m/^\\/) {
-			$diff_class = " incomplete";
+			$diff_class = "incomplete";
 		} elsif ($prefix =~ tr/+/+/) {
-			$diff_class = " add";
+			$diff_class = "add";
 		} elsif ($prefix =~ tr/-/-/) {
-			$diff_class = " rem";
+			$diff_class = "rem";
 		}
 	} else {
 		# assume ordinary diff
 		my $char = substr($line, 0, 1);
 		if ($char eq '+') {
-			$diff_class = " add";
+			$diff_class = "add";
 		} elsif ($char eq '-') {
-			$diff_class = " rem";
+			$diff_class = "rem";
 		} elsif ($char eq '@') {
-			$diff_class = " chunk_header";
+			$diff_class = "chunk_header";
 		} elsif ($char eq "\\") {
-			$diff_class = " incomplete";
+			$diff_class = "incomplete";
 		}
 	}
 	$line = untabify($line);
@@ -2274,7 +2277,7 @@ sub format_diff_line {
 		}
 		$line = "<span class=\"chunk_info\">@@ $from_text $to_text @@</span>" .
 		        "<span class=\"section\">" . esc_html($section, -nbsp=>1) .
"</span>";
-		return "<div class=\"diff$diff_class\">$line</div>\n";
+		return $diff_class, "<div class=\"diff $diff_class\">$line</div>\n";
 	} elsif ($from && $to && $line =~ m/^\@{3}/) {
 		my ($prefix, $ranges, $section) = $line =~ m/^(\@+) (.*?) \@+(.*)$/;
 		my (@from_text, @from_start, @from_nlines, $to_text, $to_start, $to_nlines);
@@ -2307,9 +2310,9 @@ sub format_diff_line {
 		}
 		$line .= " $prefix</span>" .
 		         "<span class=\"section\">" . esc_html($section, -nbsp=>1)
. "</span>";
-		return "<div class=\"diff$diff_class\">$line</div>\n";
+		return $diff_class, "<div class=\"diff $diff_class\">$line</div>\n";
 	}
-	return "<div class=\"diff$diff_class\">" . esc_html($line, -nbsp=>1)
. "</div>\n";
+	return $diff_class, "<div class=\"diff $diff_class\">" .
esc_html($line, -nbsp=>1) . "</div>\n";
 }

 # Generates undef or something like "_snapshot_" or "snapshot (_tbz2_ _zip_)",
@@ -4826,8 +4829,32 @@ sub git_difftree_body {
 	print "</table>\n";
 }

+sub format_diff_chunk {
+	my @chunk = @_;
+
+	my $first_class = $chunk[0]->[0];
+	my @partial = map { $_->[1] } grep { $_->[0] eq $first_class } @chunk;
+
+	if (scalar @partial < scalar @chunk) {
+		return join '', ("<div class='chunk'><div class='old'>",
+		                 @partial,
+		                 "</div>",
+		                 "<div class='new'>",
+		                 (map {
+		                     $_->[1];
+		                 } @chunk[scalar @partial..scalar @chunk-1]),
+		                 "</div></div>");
+	} else {
+		return join '', ("<div class='chunk'><div class='",
+		                 ($first_class eq 'add' ? 'new' : 'old'),
+		                 "'>",
+		                 @partial,
+		                 "</div></div>");
+	}
+}
+
 sub git_patchset_body {
-	my ($fd, $difftree, $hash, @hash_parents) = @_;
+	my ($fd, $is_inline, $difftree, $hash, @hash_parents) = @_;
 	my ($hash_parent) = $hash_parents[0];

 	my $is_combined = (@hash_parents > 1);
@@ -4938,12 +4965,31 @@ sub git_patchset_body {

 		# the patch itself
 	LINE:
+		my @chunk;
 		while ($patch_line = <$fd>) {
 			chomp $patch_line;

 			next PATCH if ($patch_line =~ m/^diff /);

-			print format_diff_line($patch_line, \%from, \%to);
+			my ($class, $line) = format_diff_line($patch_line, \%from, \%to);
+			if ($is_inline) {
+				print $line;
+			} elsif ($class eq 'add' || $class eq 'rem') {
+				push @chunk, [ $class, $line ];
+			} else {
+				if (@chunk) {
+					print format_diff_chunk(@chunk);
+					@chunk = ();
+				} elsif ($class eq 'chunk_header') {
+					print $line;
+				} else {
+					print q{<div class='chunk'><div class='old'>},
+					      $line,
+					      q{</div><div class='new'>},
+					      $line,
+					      q{</div></div>};
+				}
+			}
 		}

 	} continue {
@@ -7022,7 +7068,7 @@ sub git_blobdiff {
 			        "raw");
 		git_header_html(undef, $expires);
 		if (defined $hash_base && (my %co = parse_commit($hash_base))) {
-			git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
+			git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base,
$formats_nav . diff_nav($input_params{diff_style}));
 			git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
 		} else {
 			print "<div class=\"page_nav\"><br/>$formats_nav<br/></div>\n";
@@ -7051,7 +7097,7 @@ sub git_blobdiff {
 	if ($format eq 'html') {
 		print "<div class=\"page_body\">\n";

-		git_patchset_body($fd, [ \%diffinfo ], $hash_base, $hash_parent_base);
+		git_patchset_body($fd, $input_params{diff_style} eq 'inline', [
\%diffinfo ], $hash_base, $hash_parent_base);
 		close $fd;

 		print "</div>\n"; # class="page_body"
@@ -7076,6 +7122,22 @@ sub git_blobdiff_plain {
 	git_blobdiff('plain');
 }

+sub diff_nav {
+	my ($style) = @_;
+
+	my %pairs = (inline => 'inline', 'sidebyside' => 'side by side');
+	join '', ($cgi->start_form({ method => 'get' }),
+	          $cgi->hidden('p'),
+	          $cgi->hidden('a'),
+	          $cgi->hidden('h'),
+	          $cgi->hidden('hp'),
+	          $cgi->hidden('hb'),
+	          $cgi->hidden('hpb'),
+	          $cgi->popup_menu('ds', [keys %pairs], $style, \%pairs),
+	          $cgi->submit('change'),
+	          $cgi->end_form);
+}
+
 sub git_commitdiff {
 	my %params = @_;
 	my $format = $params{-format} || 'html';
@@ -7228,7 +7290,7 @@ sub git_commitdiff {
 		my $ref = format_ref_marker($refs, $co{'id'});

 		git_header_html(undef, $expires);
-		git_print_page_nav('commitdiff','', $hash,$co{'tree'},$hash, $formats_nav);
+		git_print_page_nav('commitdiff','', $hash,$co{'tree'},$hash,
$formats_nav . diff_nav($input_params{diff_style}));
 		git_print_header_div('commit', esc_html($co{'title'}) . $ref, $hash);
 		print "<div class=\"title_text\">\n" .
 		      "<table class=\"object_header\">\n";
@@ -7282,7 +7344,8 @@ sub git_commitdiff {
 		                  $use_parents ? @{$co{'parents'}} : $hash_parent);
 		print "<br/>\n";

-		git_patchset_body($fd, \@difftree, $hash,
+		git_patchset_body($fd, $input_params{diff_style} eq 'inline',
+                          \@difftree, $hash,
 		                  $use_parents ? @{$co{'parents'}} : $hash_parent);
 		close $fd;
 		print "</div>\n"; # class="page_body"
diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css
index 7d88509..a6872f9 100644
--- a/gitweb/static/gitweb.css
+++ b/gitweb/static/gitweb.css
@@ -618,6 +618,22 @@ div.remote {
 	cursor: pointer;
 }

+/* side-by-side diff */
+div.chunk {
+    overflow: hidden;
+}
+
+div.chunk div.old {
+    float: left;
+    width: 50%;
+    overflow: hidden;
+}
+
+div.chunk div.new {
+    margin-left: 50%;
+    width: 50%;
+}
+

 /* Style definition generated by highlight 2.4.5, http://www.andre-simon.de/ */

-- 
1.7.7.213.g8b0e1.dirty

^ permalink raw reply related

* Re: [PATCH] gitweb: add extensions to highlight feature map
From: Philip Oakley @ 2011-10-16 23:18 UTC (permalink / raw)
  To: Lénaïc Huard; +Cc: git
In-Reply-To: <201110161859.49817.lenaic@lhuard.fr.eu.org>

From: "Lénaïc Huard" <lenaic@lhuard.fr.eu.org>
Sent: Sunday, October 16, 2011 5:59 PM
> added: hpp, m

'm' files can be matlab files, and matlab can benefit from git's branch 
model. Matlab expectes filenames to be identical to function names(!), so 
branching function variants makes for easier development. Without git you 
can't easily refactor.
Philip

>
> Signed-off-by: Lénaïc Huard <lenaic@lhuard.fr.eu.org>
> ---
> gitweb/gitweb.perl |    3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 85d64b2..75e4854 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -262,11 +262,12 @@ our %highlight_ext = (
>        # alternate extensions, see /etc/highlight/filetypes.conf
>        'h' => 'c',
>        map { $_ => 'sh'  } qw(bash zsh ksh),
> -       map { $_ => 'cpp' } qw(cxx c++ cc),
> +       map { $_ => 'cpp' } qw(cxx c++ cc hpp),
>        map { $_ => 'php' } qw(php3 php4 php5 phps),
>        map { $_ => 'pl'  } qw(perl pm), # perhaps also 'cgi'
>        map { $_ => 'make'} qw(mak mk),
>        map { $_ => 'xml' } qw(xhtml html htm),
> +       'm' => 'objc',
> );
>
> # You define site-wide feature defaults here; override them with
> -- 
> 1.7.7
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 2012.0.1831 / Virus Database: 2090/4554 - Release Date: 10/15/11
> 

^ permalink raw reply

* Re: [PATCH 1/6] git-p4 tests: refactor and cleanup
From: Pete Wyckoff @ 2011-10-16 22:48 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Luke Diamand, Chris Li, Stefano Lattarini
In-Reply-To: <20111016144319.GD22144@arf.padd.com>

pw@padd.com wrote on Sun, 16 Oct 2011 10:43 -0400:
> Introduce a library for functions that are common to
> multiple git-p4 test files.

Update, replacing bare "python" invocation with "$PYTHON_PATH".


-------------------------8<-----------------------------

>From 3a26dbf40d9aa9f8432d994d5697ad10fce8ed91 Mon Sep 17 00:00:00 2001
From: Pete Wyckoff <pw@padd.com>
Date: Mon, 22 Aug 2011 22:20:33 -0400
Subject: [PATCH 1/6] git-p4 tests: refactor and cleanup

Introduce a library for functions that are common to
multiple git-p4 test files.

Be a bit more clever about starting and stopping p4d.
Specify a unique port number for each test, so that
tests can run in parallel.  Start p4d not in daemon mode,
and save the pid, to be able to kill it cleanly later.
Never kill p4d at startup; always shutdown cleanly.

Handle directory changes better.  Always chdir inside
a subshell, and remove any post-test directory changes.

Clean up whitespace, and use test_cmp and test_must_fail
more consistently.

Separate the tests related to detecting p4 branches
into their own file, and add a few more.

Acked-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 t/lib-git-p4.sh          |   74 +++++++
 t/t9800-git-p4.sh        |  546 ++++++++++++++++++++--------------------------
 t/t9801-git-p4-branch.sh |  233 ++++++++++++++++++++
 3 files changed, 544 insertions(+), 309 deletions(-)
 create mode 100644 t/lib-git-p4.sh
 create mode 100755 t/t9801-git-p4-branch.sh

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
new file mode 100644
index 0000000..a870f9a
--- /dev/null
+++ b/t/lib-git-p4.sh
@@ -0,0 +1,74 @@
+#
+# Library code for git-p4 tests
+#
+
+. ./test-lib.sh
+
+if ! test_have_prereq PYTHON; then
+	skip_all='skipping git-p4 tests; python not available'
+	test_done
+fi
+( p4 -h && p4d -h ) >/dev/null 2>&1 || {
+	skip_all='skipping git-p4 tests; no p4 or p4d'
+	test_done
+}
+
+GITP4="$GIT_BUILD_DIR/contrib/fast-import/git-p4"
+
+# Try to pick a unique port: guess a large number, then hope
+# no more than one of each test is running.
+#
+# This does not handle the case where somebody else is running the
+# same tests and has chosen the same ports.
+testid=${this_test#t}
+git_p4_test_start=9800
+P4DPORT=$((10669 + ($testid - $git_p4_test_start)))
+
+export P4PORT=localhost:$P4DPORT
+export P4CLIENT=client
+
+db="$TRASH_DIRECTORY/db"
+cli="$TRASH_DIRECTORY/cli"
+git="$TRASH_DIRECTORY/git"
+pidfile="$TRASH_DIRECTORY/p4d.pid"
+
+start_p4d() {
+	mkdir -p "$db" "$cli" "$git" &&
+	(
+		p4d -q -r "$db" -p $P4DPORT &
+		echo $! >"$pidfile"
+	) &&
+	for i in 1 2 3 4 5 ; do
+		p4 info >/dev/null 2>&1 && break || true &&
+		echo waiting for p4d to start &&
+		sleep 1
+	done &&
+	# complain if it never started
+	p4 info >/dev/null &&
+	(
+		cd "$cli" &&
+		p4 client -i <<-EOF
+		Client: client
+		Description: client
+		Root: $cli
+		View: //depot/... //client/...
+		EOF
+	)
+}
+
+kill_p4d() {
+	pid=$(cat "$pidfile")
+	# it had better exist for the first kill
+	kill $pid &&
+	for i in 1 2 3 4 5 ; do
+		kill $pid >/dev/null 2>&1 || break
+		sleep 1
+	done &&
+	# complain if it would not die
+	test_must_fail kill $pid >/dev/null 2>&1 &&
+	rm -rf "$db" "$cli" "$pidfile"
+}
+
+cleanup_git() {
+	rm -rf "$git"
+}
diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index 01ba041..272de3f 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -2,76 +2,51 @@
 
 test_description='git-p4 tests'
 
-. ./test-lib.sh
+. ./lib-git-p4.sh
 
-( p4 -h && p4d -h ) >/dev/null 2>&1 || {
-	skip_all='skipping git-p4 tests; no p4 or p4d'
-	test_done
-}
-
-GITP4=$GIT_BUILD_DIR/contrib/fast-import/git-p4
-P4DPORT=10669
-
-export P4PORT=localhost:$P4DPORT
-
-db="$TRASH_DIRECTORY/db"
-cli="$TRASH_DIRECTORY/cli"
-git="$TRASH_DIRECTORY/git"
-
-test_debug 'echo p4d -q -d -r "$db" -p $P4DPORT'
-test_expect_success setup '
-	mkdir -p "$db" &&
-	p4d -q -d -r "$db" -p $P4DPORT &&
-	mkdir -p "$cli" &&
-	mkdir -p "$git" &&
-	export P4PORT=localhost:$P4DPORT
+test_expect_success 'start p4d' '
+	start_p4d
 '
 
 test_expect_success 'add p4 files' '
-	cd "$cli" &&
-	p4 client -i <<-EOF &&
-	Client: client
-	Description: client
-	Root: $cli
-	View: //depot/... //client/...
-	EOF
-	export P4CLIENT=client &&
-	echo file1 >file1 &&
-	p4 add file1 &&
-	p4 submit -d "file1" &&
-	echo file2 >file2 &&
-	p4 add file2 &&
-	p4 submit -d "file2" &&
-	cd "$TRASH_DIRECTORY"
+	(
+		cd "$cli" &&
+		echo file1 >file1 &&
+		p4 add file1 &&
+		p4 submit -d "file1" &&
+		echo file2 >file2 &&
+		p4 add file2 &&
+		p4 submit -d "file2"
+	)
 '
 
-cleanup_git() {
-	cd "$TRASH_DIRECTORY" &&
-	rm -rf "$git" &&
-	mkdir "$git"
-}
-
 test_expect_success 'basic git-p4 clone' '
 	"$GITP4" clone --dest="$git" //depot &&
 	test_when_finished cleanup_git &&
-	cd "$git" &&
-	git log --oneline >lines &&
-	test_line_count = 1 lines
+	(
+		cd "$git" &&
+		git log --oneline >lines &&
+		test_line_count = 1 lines
+	)
 '
 
 test_expect_success 'git-p4 clone @all' '
 	"$GITP4" clone --dest="$git" //depot@all &&
 	test_when_finished cleanup_git &&
-	cd "$git" &&
-	git log --oneline >lines &&
-	test_line_count = 2 lines
+	(
+		cd "$git" &&
+		git log --oneline >lines &&
+		test_line_count = 2 lines
+	)
 '
 
 test_expect_success 'git-p4 sync uninitialized repo' '
 	test_create_repo "$git" &&
 	test_when_finished cleanup_git &&
-	cd "$git" &&
-	test_must_fail "$GITP4" sync
+	(
+		cd "$git" &&
+		test_must_fail "$GITP4" sync
+	)
 '
 
 #
@@ -81,17 +56,19 @@ test_expect_success 'git-p4 sync uninitialized repo' '
 test_expect_success 'git-p4 sync new branch' '
 	test_create_repo "$git" &&
 	test_when_finished cleanup_git &&
-	cd "$git" &&
-	test_commit head &&
-	"$GITP4" sync --branch=refs/remotes/p4/depot //depot@all &&
-	git log --oneline p4/depot >lines &&
-	test_line_count = 2 lines
+	(
+		cd "$git" &&
+		test_commit head &&
+		"$GITP4" sync --branch=refs/remotes/p4/depot //depot@all &&
+		git log --oneline p4/depot >lines &&
+		test_line_count = 2 lines
+	)
 '
 
 test_expect_success 'exit when p4 fails to produce marshaled output' '
 	badp4dir="$TRASH_DIRECTORY/badp4dir" &&
-	mkdir -p "$badp4dir" &&
-	test_when_finished "rm -rf $badp4dir" &&
+	mkdir "$badp4dir" &&
+	test_when_finished "rm \"$badp4dir/p4\" && rmdir \"$badp4dir\"" &&
 	cat >"$badp4dir"/p4 <<-EOF &&
 	#!$SHELL_PATH
 	exit 1
@@ -103,61 +80,61 @@ test_expect_success 'exit when p4 fails to produce marshaled output' '
 '
 
 test_expect_success 'add p4 files with wildcards in the names' '
-	cd "$cli" &&
-	echo file-wild-hash >file-wild#hash &&
-	echo file-wild-star >file-wild\*star &&
-	echo file-wild-at >file-wild@at &&
-	echo file-wild-percent >file-wild%percent &&
-	p4 add -f file-wild* &&
-	p4 submit -d "file wildcards"
+	(
+		cd "$cli" &&
+		echo file-wild-hash >file-wild#hash &&
+		echo file-wild-star >file-wild\*star &&
+		echo file-wild-at >file-wild@at &&
+		echo file-wild-percent >file-wild%percent &&
+		p4 add -f file-wild* &&
+		p4 submit -d "file wildcards"
+	)
 '
 
 test_expect_success 'wildcard files git-p4 clone' '
 	"$GITP4" clone --dest="$git" //depot &&
 	test_when_finished cleanup_git &&
-	cd "$git" &&
-	test -f file-wild#hash &&
-	test -f file-wild\*star &&
-	test -f file-wild@at &&
-	test -f file-wild%percent
+	(
+		cd "$git" &&
+		test -f file-wild#hash &&
+		test -f file-wild\*star &&
+		test -f file-wild@at &&
+		test -f file-wild%percent
+	)
 '
 
 test_expect_success 'clone bare' '
 	"$GITP4" clone --dest="$git" --bare //depot &&
 	test_when_finished cleanup_git &&
-	cd "$git" &&
-	test ! -d .git &&
-	bare=`git config --get core.bare` &&
-	test "$bare" = true
+	(
+		cd "$git" &&
+		test ! -d .git &&
+		bare=`git config --get core.bare` &&
+		test "$bare" = true
+	)
 '
 
 p4_add_user() {
-    name=$1
-    fullname=$2
-    p4 user -f -i <<EOF &&
-User: $name
-Email: $name@localhost
-FullName: $fullname
-EOF
-    p4 passwd -P secret $name
+	name=$1 fullname=$2 &&
+	p4 user -f -i <<-EOF &&
+	User: $name
+	Email: $name@localhost
+	FullName: $fullname
+	EOF
+	p4 passwd -P secret $name
 }
 
 p4_grant_admin() {
-    name=$1
-    p4 protect -o |\
-	awk "{print}END{print \"    admin user $name * //depot/...\"}" |\
-	p4 protect -i
+	name=$1 &&
+	{
+		p4 protect -o &&
+		echo "    admin user $name * //depot/..."
+	} | p4 protect -i
 }
 
 p4_check_commit_author() {
-    file=$1
-    user=$2
-    if p4 changes -m 1 //depot/$file | grep $user > /dev/null ; then
-	return 0
-    else
-	echo "file $file not modified by user $user" 1>&2
-	return 1
-    fi
+	file=$1 user=$2 &&
+	p4 changes -m 1 //depot/$file | grep -q $user
 }
 
 make_change_by_user() {
@@ -174,15 +151,17 @@ test_expect_success 'preserve users' '
 	p4_grant_admin alice &&
 	"$GITP4" clone --dest="$git" //depot &&
 	test_when_finished cleanup_git &&
-	cd "$git" &&
-	echo "username: a change by alice" >> file1 &&
-	echo "username: a change by bob" >> file2 &&
-	git commit --author "Alice <alice@localhost>" -m "a change by alice" file1 &&
-	git commit --author "Bob <bob@localhost>" -m "a change by bob" file2 &&
-	git config git-p4.skipSubmitEditCheck true &&
-	P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit --preserve-user &&
-	p4_check_commit_author file1 alice &&
-	p4_check_commit_author file2 bob
+	(
+		cd "$git" &&
+		echo "username: a change by alice" >>file1 &&
+		echo "username: a change by bob" >>file2 &&
+		git commit --author "Alice <alice@localhost>" -m "a change by alice" file1 &&
+		git commit --author "Bob <bob@localhost>" -m "a change by bob" file2 &&
+		git config git-p4.skipSubmitEditCheck true &&
+		P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit --preserve-user &&
+		p4_check_commit_author file1 alice &&
+		p4_check_commit_author file2 bob
+	)
 '
 
 # Test username support, submitting as bob, who lacks admin rights. Should
@@ -190,32 +169,37 @@ test_expect_success 'preserve users' '
 test_expect_success 'refuse to preserve users without perms' '
 	"$GITP4" clone --dest="$git" //depot &&
 	test_when_finished cleanup_git &&
-	cd "$git" &&
-	git config git-p4.skipSubmitEditCheck true &&
-	echo "username-noperms: a change by alice" >> file1 &&
-	git commit --author "Alice <alice@localhost>" -m "perms: a change by alice" file1 &&
-	! P4EDITOR=touch P4USER=bob P4PASSWD=secret "$GITP4" commit --preserve-user &&
-	! git diff --exit-code HEAD..p4/master > /dev/null
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEditCheck true &&
+		echo "username-noperms: a change by alice" >>file1 &&
+		git commit --author "Alice <alice@localhost>" -m "perms: a change by alice" file1 &&
+		P4EDITOR=touch P4USER=bob P4PASSWD=secret test_must_fail "$GITP4" commit --preserve-user &&
+		test_must_fail git diff --exit-code HEAD..p4/master
+	)
 '
 
 # What happens with unknown author? Without allowMissingP4Users it should fail.
 test_expect_success 'preserve user where author is unknown to p4' '
 	"$GITP4" clone --dest="$git" //depot &&
 	test_when_finished cleanup_git &&
-	cd "$git" &&
-	git config git-p4.skipSubmitEditCheck true &&
-	echo "username-bob: a change by bob" >> file1 &&
-	git commit --author "Bob <bob@localhost>" -m "preserve: a change by bob" file1 &&
-	echo "username-unknown: a change by charlie" >> file1 &&
-	git commit --author "Charlie <charlie@localhost>" -m "preserve: a change by charlie" file1 &&
-	! P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit --preserve-user &&
-	! git diff --exit-code HEAD..p4/master > /dev/null &&
-	echo "$0: repeat with allowMissingP4Users enabled" &&
-	git config git-p4.allowMissingP4Users true &&
-	git config git-p4.preserveUser true &&
-	P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit &&
-	git diff --exit-code HEAD..p4/master > /dev/null &&
-	p4_check_commit_author file1 alice
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEditCheck true &&
+		echo "username-bob: a change by bob" >>file1 &&
+		git commit --author "Bob <bob@localhost>" -m "preserve: a change by bob" file1 &&
+		echo "username-unknown: a change by charlie" >>file1 &&
+		git commit --author "Charlie <charlie@localhost>" -m "preserve: a change by charlie" file1 &&
+		P4EDITOR=touch P4USER=alice P4PASSWD=secret test_must_fail "$GITP4" commit --preserve-user &&
+		test_must_fail git diff --exit-code HEAD..p4/master &&
+
+		echo "$0: repeat with allowMissingP4Users enabled" &&
+		git config git-p4.allowMissingP4Users true &&
+		git config git-p4.preserveUser true &&
+		P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit &&
+		git diff --exit-code HEAD..p4/master &&
+		p4_check_commit_author file1 alice
+	)
 '
 
 # If we're *not* using --preserve-user, git-p4 should warn if we're submitting
@@ -225,33 +209,35 @@ test_expect_success 'preserve user where author is unknown to p4' '
 test_expect_success 'not preserving user with mixed authorship' '
 	"$GITP4" clone --dest="$git" //depot &&
 	test_when_finished cleanup_git &&
-	cd "$git" &&
-	git config git-p4.skipSubmitEditCheck true &&
-	p4_add_user derek Derek &&
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEditCheck true &&
+		p4_add_user derek Derek &&
 
-	make_change_by_user usernamefile3 Derek derek@localhost &&
-	P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
-	grep "git author derek@localhost does not match" actual &&
+		make_change_by_user usernamefile3 Derek derek@localhost &&
+		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit |\
+		grep "git author derek@localhost does not match" &&
 
-	make_change_by_user usernamefile3 Charlie charlie@localhost &&
-	P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
-	grep "git author charlie@localhost does not match" actual &&
+		make_change_by_user usernamefile3 Charlie charlie@localhost &&
+		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit |\
+		grep "git author charlie@localhost does not match" &&
 
-	make_change_by_user usernamefile3 alice alice@localhost &&
-	P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
-	! grep "git author.*does not match" actual &&
+		make_change_by_user usernamefile3 alice alice@localhost &&
+		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" |\
+		test_must_fail grep "git author.*does not match" &&
 
-	git config git-p4.skipUserNameCheck true &&
-	make_change_by_user usernamefile3 Charlie charlie@localhost &&
-	P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
-	! grep "git author.*does not match" actual &&
+		git config git-p4.skipUserNameCheck true &&
+		make_change_by_user usernamefile3 Charlie charlie@localhost &&
+		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit |\
+		test_must_fail grep "git author.*does not match" &&
 
-	p4_check_commit_author usernamefile3 alice
+		p4_check_commit_author usernamefile3 alice
+	)
 '
 
 marshal_dump() {
 	what=$1
-	python -c 'import marshal, sys; d = marshal.load(sys.stdin); print d["'$what'"]'
+	"$PYTHON_PATH" -c 'import marshal, sys; d = marshal.load(sys.stdin); print d["'$what'"]'
 }
 
 # Sleep a bit so that the top-most p4 change did not happen "now".  Then
@@ -263,10 +249,12 @@ test_expect_success 'initial import time from top change time' '
 	sleep 3 &&
 	"$GITP4" clone --dest="$git" //depot &&
 	test_when_finished cleanup_git &&
-	cd "$git" &&
-	gittime=$(git show -s --raw --pretty=format:%at HEAD) &&
-	echo $p4time $gittime &&
-	test $p4time = $gittime
+	(
+		cd "$git" &&
+		gittime=$(git show -s --raw --pretty=format:%at HEAD) &&
+		echo $p4time $gittime &&
+		test $p4time = $gittime
+	)
 '
 
 # Rename a file and confirm that rename is not detected in P4.
@@ -279,47 +267,49 @@ test_expect_success 'initial import time from top change time' '
 test_expect_success 'detect renames' '
 	"$GITP4" clone --dest="$git" //depot@all &&
 	test_when_finished cleanup_git &&
-	cd "$git" &&
-	git config git-p4.skipSubmitEditCheck true &&
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEditCheck true &&
 
-	git mv file1 file4 &&
-	git commit -a -m "Rename file1 to file4" &&
-	git diff-tree -r -M HEAD &&
-	"$GITP4" submit &&
-	p4 filelog //depot/file4 &&
-	! p4 filelog //depot/file4 | grep -q "branch from" &&
+		git mv file1 file4 &&
+		git commit -a -m "Rename file1 to file4" &&
+		git diff-tree -r -M HEAD &&
+		"$GITP4" submit &&
+		p4 filelog //depot/file4 &&
+		p4 filelog //depot/file4 | test_must_fail grep -q "branch from" &&
 
-	git mv file4 file5 &&
-	git commit -a -m "Rename file4 to file5" &&
-	git diff-tree -r -M HEAD &&
-	git config git-p4.detectRenames true &&
-	"$GITP4" submit &&
-	p4 filelog //depot/file5 &&
-	p4 filelog //depot/file5 | grep -q "branch from //depot/file4" &&
+		git mv file4 file5 &&
+		git commit -a -m "Rename file4 to file5" &&
+		git diff-tree -r -M HEAD &&
+		git config git-p4.detectRenames true &&
+		"$GITP4" submit &&
+		p4 filelog //depot/file5 &&
+		p4 filelog //depot/file5 | grep -q "branch from //depot/file4" &&
 
-	git mv file5 file6 &&
-	echo update >>file6 &&
-	git add file6 &&
-	git commit -a -m "Rename file5 to file6 with changes" &&
-	git diff-tree -r -M HEAD &&
-	level=$(git diff-tree -r -M HEAD | sed 1d | cut -f1 | cut -d" " -f5 | sed "s/R0*//") &&
-	test -n "$level" && test "$level" -gt 0 && test "$level" -lt 98 &&
-	git config git-p4.detectRenames $((level + 2)) &&
-	"$GITP4" submit &&
-	p4 filelog //depot/file6 &&
-	! p4 filelog //depot/file6 | grep -q "branch from" &&
+		git mv file5 file6 &&
+		echo update >>file6 &&
+		git add file6 &&
+		git commit -a -m "Rename file5 to file6 with changes" &&
+		git diff-tree -r -M HEAD &&
+		level=$(git diff-tree -r -M HEAD | sed 1d | cut -f1 | cut -d" " -f5 | sed "s/R0*//") &&
+		test -n "$level" && test "$level" -gt 0 && test "$level" -lt 98 &&
+		git config git-p4.detectRenames $(($level + 2)) &&
+		"$GITP4" submit &&
+		p4 filelog //depot/file6 &&
+		p4 filelog //depot/file6 | test_must_fail grep -q "branch from" &&
 
-	git mv file6 file7 &&
-	echo update >>file7 &&
-	git add file7 &&
-	git commit -a -m "Rename file6 to file7 with changes" &&
-	git diff-tree -r -M HEAD &&
-	level=$(git diff-tree -r -M HEAD | sed 1d | cut -f1 | cut -d" " -f5 | sed "s/R0*//") &&
-	test -n "$level" && test "$level" -gt 2 && test "$level" -lt 100 &&
-	git config git-p4.detectRenames $((level - 2)) &&
-	"$GITP4" submit &&
-	p4 filelog //depot/file7 &&
-	p4 filelog //depot/file7 | grep -q "branch from //depot/file6"
+		git mv file6 file7 &&
+		echo update >>file7 &&
+		git add file7 &&
+		git commit -a -m "Rename file6 to file7 with changes" &&
+		git diff-tree -r -M HEAD &&
+		level=$(git diff-tree -r -M HEAD | sed 1d | cut -f1 | cut -d" " -f5 | sed "s/R0*//") &&
+		test -n "$level" && test "$level" -gt 2 && test "$level" -lt 100 &&
+		git config git-p4.detectRenames $(($level - 2)) &&
+		"$GITP4" submit &&
+		p4 filelog //depot/file7 &&
+		p4 filelog //depot/file7 | grep -q "branch from //depot/file6"
+	)
 '
 
 # Copy a file and confirm that copy is not detected in P4.
@@ -336,141 +326,79 @@ test_expect_success 'detect renames' '
 test_expect_success 'detect copies' '
 	"$GITP4" clone --dest="$git" //depot@all &&
 	test_when_finished cleanup_git &&
-	cd "$git" &&
-	git config git-p4.skipSubmitEditCheck true &&
-
-	cp file2 file8 &&
-	git add file8 &&
-	git commit -a -m "Copy file2 to file8" &&
-	git diff-tree -r -C HEAD &&
-	"$GITP4" submit &&
-	p4 filelog //depot/file8 &&
-	! p4 filelog //depot/file8 | grep -q "branch from" &&
-
-	cp file2 file9 &&
-	git add file9 &&
-	git commit -a -m "Copy file2 to file9" &&
-	git diff-tree -r -C HEAD &&
-	git config git-p4.detectCopies true &&
-	"$GITP4" submit &&
-	p4 filelog //depot/file9 &&
-	! p4 filelog //depot/file9 | grep -q "branch from" &&
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEditCheck true &&
 
-	echo "file2" >>file2 &&
-	cp file2 file10 &&
-	git add file2 file10 &&
-	git commit -a -m "Modify and copy file2 to file10" &&
-	git diff-tree -r -C HEAD &&
-	"$GITP4" submit &&
-	p4 filelog //depot/file10 &&
-	p4 filelog //depot/file10 | grep -q "branch from //depot/file" &&
+		cp file2 file8 &&
+		git add file8 &&
+		git commit -a -m "Copy file2 to file8" &&
+		git diff-tree -r -C HEAD &&
+		"$GITP4" submit &&
+		p4 filelog //depot/file8 &&
+		p4 filelog //depot/file8 | test_must_fail grep -q "branch from" &&
 
-	cp file2 file11 &&
-	git add file11 &&
-	git commit -a -m "Copy file2 to file11" &&
-	git diff-tree -r -C --find-copies-harder HEAD &&
-	src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
-	test "$src" = file10 &&
-	git config git-p4.detectCopiesHarder true &&
-	"$GITP4" submit &&
-	p4 filelog //depot/file11 &&
-	p4 filelog //depot/file11 | grep -q "branch from //depot/file" &&
+		cp file2 file9 &&
+		git add file9 &&
+		git commit -a -m "Copy file2 to file9" &&
+		git diff-tree -r -C HEAD &&
+		git config git-p4.detectCopies true &&
+		"$GITP4" submit &&
+		p4 filelog //depot/file9 &&
+		p4 filelog //depot/file9 | test_must_fail grep -q "branch from" &&
 
-	cp file2 file12 &&
-	echo "some text" >>file12 &&
-	git add file12 &&
-	git commit -a -m "Copy file2 to file12 with changes" &&
-	git diff-tree -r -C --find-copies-harder HEAD &&
-	level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d" " -f5 | sed "s/C0*//") &&
-	test -n "$level" && test "$level" -gt 0 && test "$level" -lt 98 &&
-	src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
-	test "$src" = file10 &&
-	git config git-p4.detectCopies $((level + 2)) &&
-	"$GITP4" submit &&
-	p4 filelog //depot/file12 &&
-	! p4 filelog //depot/file12 | grep -q "branch from" &&
+		echo "file2" >>file2 &&
+		cp file2 file10 &&
+		git add file2 file10 &&
+		git commit -a -m "Modify and copy file2 to file10" &&
+		git diff-tree -r -C HEAD &&
+		"$GITP4" submit &&
+		p4 filelog //depot/file10 &&
+		p4 filelog //depot/file10 | grep -q "branch from //depot/file" &&
 
-	cp file2 file13 &&
-	echo "different text" >>file13 &&
-	git add file13 &&
-	git commit -a -m "Copy file2 to file13 with changes" &&
-	git diff-tree -r -C --find-copies-harder HEAD &&
-	level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d" " -f5 | sed "s/C0*//") &&
-	test -n "$level" && test "$level" -gt 2 && test "$level" -lt 100 &&
-	src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
-	test "$src" = file10 &&
-	git config git-p4.detectCopies $((level - 2)) &&
-	"$GITP4" submit &&
-	p4 filelog //depot/file13 &&
-	p4 filelog //depot/file13 | grep -q "branch from //depot/file"
-'
+		cp file2 file11 &&
+		git add file11 &&
+		git commit -a -m "Copy file2 to file11" &&
+		git diff-tree -r -C --find-copies-harder HEAD &&
+		src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
+		test "$src" = file10 &&
+		git config git-p4.detectCopiesHarder true &&
+		"$GITP4" submit &&
+		p4 filelog //depot/file11 &&
+		p4 filelog //depot/file11 | grep -q "branch from //depot/file" &&
 
-# Create a simple branch structure in P4 depot to check if it is correctly
-# cloned.
-test_expect_success 'add simple p4 branches' '
-	cd "$cli" &&
-	mkdir branch1 &&
-	cd branch1 &&
-	echo file1 >file1 &&
-	echo file2 >file2 &&
-	p4 add file1 file2 &&
-	p4 submit -d "branch1" &&
-	p4 integrate //depot/branch1/... //depot/branch2/... &&
-	p4 submit -d "branch2" &&
-	echo file3 >file3 &&
-	p4 add file3 &&
-	p4 submit -d "add file3 in branch1" &&
-	p4 open file2 &&
-	echo update >>file2 &&
-	p4 submit -d "update file2 in branch1" &&
-	p4 integrate //depot/branch1/... //depot/branch3/... &&
-	p4 submit -d "branch3" &&
-	cd "$TRASH_DIRECTORY"
-'
+		cp file2 file12 &&
+		echo "some text" >>file12 &&
+		git add file12 &&
+		git commit -a -m "Copy file2 to file12 with changes" &&
+		git diff-tree -r -C --find-copies-harder HEAD &&
+		level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d" " -f5 | sed "s/C0*//") &&
+		test -n "$level" && test "$level" -gt 0 && test "$level" -lt 98 &&
+		src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
+		test "$src" = file10 &&
+		git config git-p4.detectCopies $(($level + 2)) &&
+		"$GITP4" submit &&
+		p4 filelog //depot/file12 &&
+		p4 filelog //depot/file12 | test_must_fail grep -q "branch from" &&
 
-# Configure branches through git-config and clone them.
-# All files are tested to make sure branches were cloned correctly.
-# Finally, make an update to branch1 on P4 side to check if it is imported
-# correctly by git-p4.
-test_expect_success 'git-p4 clone simple branches' '
-	test_when_finished cleanup_git &&
-	test_create_repo "$git" &&
-	cd "$git" &&
-	git config git-p4.branchList branch1:branch2 &&
-	git config --add git-p4.branchList branch1:branch3 &&
-	"$GITP4" clone --dest=. --detect-branches //depot@all &&
-	git log --all --graph --decorate --stat &&
-	git reset --hard p4/depot/branch1 &&
-	test -f file1 &&
-	test -f file2 &&
-	test -f file3 &&
-	grep -q update file2 &&
-	git reset --hard p4/depot/branch2 &&
-	test -f file1 &&
-	test -f file2 &&
-	test ! -f file3 &&
-	! grep -q update file2 &&
-	git reset --hard p4/depot/branch3 &&
-	test -f file1 &&
-	test -f file2 &&
-	test -f file3 &&
-	grep -q update file2 &&
-	cd "$cli" &&
-	cd branch1 &&
-	p4 edit file2 &&
-	echo file2_ >>file2 &&
-	p4 submit -d "update file2 in branch1" &&
-	cd "$git" &&
-	git reset --hard p4/depot/branch1 &&
-	"$GITP4" rebase &&
-	grep -q file2_ file2
+		cp file2 file13 &&
+		echo "different text" >>file13 &&
+		git add file13 &&
+		git commit -a -m "Copy file2 to file13 with changes" &&
+		git diff-tree -r -C --find-copies-harder HEAD &&
+		level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d" " -f5 | sed "s/C0*//") &&
+		test -n "$level" && test "$level" -gt 2 && test "$level" -lt 100 &&
+		src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
+		test "$src" = file10 &&
+		git config git-p4.detectCopies $(($level - 2)) &&
+		"$GITP4" submit &&
+		p4 filelog //depot/file13 &&
+		p4 filelog //depot/file13 | grep -q "branch from //depot/file"
+	)
 '
 
-test_expect_success 'shutdown' '
-	pid=`pgrep -f p4d` &&
-	test -n "$pid" &&
-	test_debug "ps wl `echo $pid`" &&
-	kill $pid
+test_expect_success 'kill p4d' '
+	kill_p4d
 '
 
 test_done
diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
new file mode 100755
index 0000000..a25f18d
--- /dev/null
+++ b/t/t9801-git-p4-branch.sh
@@ -0,0 +1,233 @@
+#!/bin/sh
+
+test_description='git-p4 p4 branching tests'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+	start_p4d
+'
+
+#
+# 1: //depot/main/f1
+# 2: //depot/main/f2
+# 3: integrate //depot/main/... -> //depot/branch1/...
+# 4: //depot/main/f4
+# 5: //depot/branch1/f5
+# .: named branch branch2
+# 6: integrate -b branch2
+# 7: //depot/branch2/f7
+# 8: //depot/main/f8
+#
+test_expect_success 'basic p4 branches' '
+	(
+		cd "$cli" &&
+		mkdir -p main &&
+
+		echo f1 >main/f1 &&
+		p4 add main/f1 &&
+		p4 submit -d "main/f1" &&
+
+		echo f2 >main/f2 &&
+		p4 add main/f2 &&
+		p4 submit -d "main/f2" &&
+
+		p4 integrate //depot/main/... //depot/branch1/... &&
+		p4 submit -d "integrate main to branch1" &&
+
+		echo f4 >main/f4 &&
+		p4 add main/f4 &&
+		p4 submit -d "main/f4" &&
+
+		echo f5 >branch1/f5 &&
+		p4 add branch1/f5 &&
+		p4 submit -d "branch1/f5" &&
+
+		p4 branch -i <<-EOF &&
+		Branch: branch2
+		View: //depot/main/... //depot/branch2/...
+		EOF
+
+		p4 integrate -b branch2 &&
+		p4 submit -d "integrate main to branch2" &&
+
+		echo f7 >branch2/f7 &&
+		p4 add branch2/f7 &&
+		p4 submit -d "branch2/f7" &&
+
+		echo f8 >main/f8 &&
+		p4 add main/f8 &&
+		p4 submit -d "main/f8"
+	)
+'
+
+test_expect_success 'import main, no branch detection' '
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --dest="$git" //depot/main@all &&
+	(
+		cd "$git" &&
+		git log --oneline --graph --decorate --all &&
+		git rev-list master >wc &&
+		test_line_count = 4 wc
+	)
+'
+
+test_expect_success 'import branch1, no branch detection' '
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --dest="$git" //depot/branch1@all &&
+	(
+		cd "$git" &&
+		git log --oneline --graph --decorate --all &&
+		git rev-list master >wc &&
+		test_line_count = 2 wc
+	)
+'
+
+test_expect_success 'import branch2, no branch detection' '
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --dest="$git" //depot/branch2@all &&
+	(
+		cd "$git" &&
+		git log --oneline --graph --decorate --all &&
+		git rev-list master >wc &&
+		test_line_count = 2 wc
+	)
+'
+
+test_expect_success 'import depot, no branch detection' '
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --dest="$git" //depot@all &&
+	(
+		cd "$git" &&
+		git log --oneline --graph --decorate --all &&
+		git rev-list master >wc &&
+		test_line_count = 8 wc
+	)
+'
+
+test_expect_success 'import depot, branch detection' '
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --dest="$git" --detect-branches //depot@all &&
+	(
+		cd "$git" &&
+
+		git log --oneline --graph --decorate --all &&
+
+		# 4 main commits
+		git rev-list master >wc &&
+		test_line_count = 4 wc &&
+
+		# 3 main, 1 integrate, 1 on branch2
+		git rev-list p4/depot/branch2 >wc &&
+		test_line_count = 5 wc &&
+
+		# no branch1, since no p4 branch created for it
+		test_must_fail git show-ref p4/depot/branch1
+	)
+'
+
+test_expect_success 'import depot, branch detection, branchList branch definition' '
+	test_when_finished cleanup_git &&
+	test_create_repo "$git" &&
+	(
+		cd "$git" &&
+		git config git-p4.branchList main:branch1 &&
+		"$GITP4" clone --dest=. --detect-branches //depot@all &&
+
+		git log --oneline --graph --decorate --all &&
+
+		# 4 main commits
+		git rev-list master >wc &&
+		test_line_count = 4 wc &&
+
+		# 3 main, 1 integrate, 1 on branch2
+		git rev-list p4/depot/branch2 >wc &&
+		test_line_count = 5 wc &&
+
+		# 2 main, 1 integrate, 1 on branch1
+		git rev-list p4/depot/branch1 >wc &&
+		test_line_count = 4 wc
+	)
+'
+
+test_expect_success 'restart p4d' '
+	kill_p4d &&
+	start_p4d
+'
+
+#
+# 1: //depot/branch1/file1
+#    //depot/branch1/file2
+# 2: integrate //depot/branch1/... -> //depot/branch2/...
+# 3: //depot/branch1/file3
+# 4: //depot/branch1/file2 (edit)
+# 5: integrate //depot/branch1/... -> //depot/branch3/...
+#
+## Create a simple branch structure in P4 depot.
+test_expect_success 'add simple p4 branches' '
+	(
+		cd "$cli" &&
+		mkdir branch1 &&
+		cd branch1 &&
+		echo file1 >file1 &&
+		echo file2 >file2 &&
+		p4 add file1 file2 &&
+		p4 submit -d "branch1" &&
+		p4 integrate //depot/branch1/... //depot/branch2/... &&
+		p4 submit -d "branch2" &&
+		echo file3 >file3 &&
+		p4 add file3 &&
+		p4 submit -d "add file3 in branch1" &&
+		p4 open file2 &&
+		echo update >>file2 &&
+		p4 submit -d "update file2 in branch1" &&
+		p4 integrate //depot/branch1/... //depot/branch3/... &&
+		p4 submit -d "branch3"
+	)
+'
+
+# Configure branches through git-config and clone them.
+# All files are tested to make sure branches were cloned correctly.
+# Finally, make an update to branch1 on P4 side to check if it is imported
+# correctly by git-p4.
+test_expect_success 'git-p4 clone simple branches' '
+	test_when_finished cleanup_git &&
+	test_create_repo "$git" &&
+	(
+		cd "$git" &&
+		git config git-p4.branchList branch1:branch2 &&
+		git config --add git-p4.branchList branch1:branch3 &&
+		"$GITP4" clone --dest=. --detect-branches //depot@all &&
+		git log --all --graph --decorate --stat &&
+		git reset --hard p4/depot/branch1 &&
+		test -f file1 &&
+		test -f file2 &&
+		test -f file3 &&
+		grep -q update file2 &&
+		git reset --hard p4/depot/branch2 &&
+		test -f file1 &&
+		test -f file2 &&
+		test ! -f file3 &&
+		test_must_fail grep -q update file2 &&
+		git reset --hard p4/depot/branch3 &&
+		test -f file1 &&
+		test -f file2 &&
+		test -f file3 &&
+		grep -q update file2 &&
+		cd "$cli" &&
+		cd branch1 &&
+		p4 edit file2 &&
+		echo file2_ >>file2 &&
+		p4 submit -d "update file2 in branch3" &&
+		cd "$git" &&
+		git reset --hard p4/depot/branch1 &&
+		"$GITP4" rebase &&
+		grep -q file2_ file2
+	)
+'
+
+test_expect_success 'kill p4d' '
+	kill_p4d
+'
+
+test_done
-- 
1.7.7

^ permalink raw reply related

* Re: [PATCH 2/6] git-p4: handle utf16 filetype properly
From: Pete Wyckoff @ 2011-10-16 22:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Luke Diamand, Chris Li, Stefano Lattarini
In-Reply-To: <7vaa90reqm.fsf@alter.siamese.dyndns.org>

gitster@pobox.com wrote on Sun, 16 Oct 2011 11:08 -0700:
> Pete Wyckoff <pw@padd.com> writes:
> > +		python "$TRASH_DIRECTORY/k_smush.py" <"$cli/k-text-k" >cli-k-text-k-smush &&
> 
> Shouldn't this pay attention to PYTHON_PATH in any way?

Yes.  I did not notice that the configuration variable existed.
I'll respond to 1/6 to fix that one too.  Below is the new 2/6,
please.  Includes the extra \n that Stefano noticed too.

		-- Pete

--------------------8<--------------------------
From f177a75182576251ae4c895861d1e0f1f92794fc Mon Sep 17 00:00:00 2001
From: Pete Wyckoff <pw@padd.com>
Date: Sat, 17 Sep 2011 19:16:14 -0400
Subject: [PATCH 2/6] git-p4: handle utf16 filetype properly

One of the filetypes that p4 supports is utf16.  Its behavior is
odd in this case.  The data delivered through "p4 -G print" is
not encoded in utf16, although "p4 print -o" will produce the
proper utf16-encoded file.

When dealing with this filetype, discard the data from -G, and
instead read the contents directly.

An alternate approach would be to try to encode the data in
python.  That worked for true utf16 files, but for other files
marked as utf16, p4 delivers mangled text in no recognizable encoding.

Add a test case to check utf16 handling, and +k and +ko handling.

Reported-by: Chris Li <git@chrisli.org>
Acked-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 contrib/fast-import/git-p4 |   11 +++++
 t/t9802-git-p4-filetype.sh |  108 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+), 0 deletions(-)
 create mode 100755 t/t9802-git-p4-filetype.sh

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 2f7b270..e69caf3 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1238,6 +1238,15 @@ class P4Sync(Command, P4UserMap):
             data = ''.join(contents)
             contents = [data[:-1]]
 
+        if file['type'].startswith("utf16"):
+            # p4 delivers different text in the python output to -G
+            # than it does when using "print -o", or normal p4 client
+            # operations.  utf16 is converted to ascii or utf8, perhaps.
+            # But ascii text saved as -t utf16 is completely mangled.
+            # Invoke print -o to get the real contents.
+            text = p4_read_pipe('print -q -o - "%s"' % file['depotFile'])
+            contents = [ text ]
+
         if self.isWindows and file["type"].endswith("text"):
             mangled = []
             for data in contents:
@@ -1245,6 +1254,8 @@ class P4Sync(Command, P4UserMap):
                 mangled.append(data)
             contents = mangled
 
+        # Note that we do not try to de-mangle keywords on utf16 files,
+        # even though in theory somebody may want that.
         if file['type'] in ('text+ko', 'unicode+ko', 'binary+ko'):
             contents = map(lambda text: re.sub(r'(?i)\$(Id|Header):[^$]*\$',r'$\1$', text), contents)
         elif file['type'] in ('text+k', 'ktext', 'kxtext', 'unicode+k', 'binary+k'):
diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
new file mode 100755
index 0000000..3b358ef
--- /dev/null
+++ b/t/t9802-git-p4-filetype.sh
@@ -0,0 +1,108 @@
+#!/bin/sh
+
+test_description='git-p4 p4 filetype tests'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+	start_p4d
+'
+
+test_expect_success 'utf-16 file create' '
+	(
+		cd "$cli" &&
+
+		# p4 saves this verbatim
+		printf "three\nline\ntext\n" >f-ascii &&
+		p4 add -t text f-ascii &&
+
+		# p4 adds \377\376 header
+		cp f-ascii f-ascii-as-utf16 &&
+		p4 add -t utf16 f-ascii-as-utf16 &&
+
+		# p4 saves this exactly as iconv produced it
+		printf "three\nline\ntext\n" | iconv -f ascii -t utf-16 >f-utf16 &&
+		p4 add -t utf16 f-utf16 &&
+
+		# this also is unchanged
+		cp f-utf16 f-utf16-as-text &&
+		p4 add -t text f-utf16-as-text &&
+
+		p4 submit -d "f files" &&
+
+		# force update of client files
+		p4 sync -f
+	)
+'
+
+test_expect_success 'utf-16 file test' '
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --dest="$git" //depot@all &&
+	(
+		cd "$git" &&
+
+		test_cmp "$cli/f-ascii" f-ascii &&
+		test_cmp "$cli/f-ascii-as-utf16" f-ascii-as-utf16 &&
+		test_cmp "$cli/f-utf16" f-utf16 &&
+		test_cmp "$cli/f-utf16-as-text" f-utf16-as-text
+	)
+'
+
+test_expect_success 'keyword file create' '
+	(
+		cd "$cli" &&
+
+		printf "id\n\$Id\$\n\$Author\$\ntext\n" >k-text-k &&
+		p4 add -t text+k k-text-k &&
+
+		cp k-text-k k-text-ko &&
+		p4 add -t text+ko k-text-ko &&
+
+		cat k-text-k | iconv -f ascii -t utf-16 >k-utf16-k &&
+		p4 add -t utf16+k k-utf16-k &&
+
+		cp k-utf16-k k-utf16-ko &&
+		p4 add -t utf16+ko k-utf16-ko &&
+
+		p4 submit -d "k files" &&
+		p4 sync -f
+	)
+'
+
+build_smush() {
+	cat >k_smush.py <<-\EOF &&
+	import re, sys
+	sys.stdout.write(re.sub(r'(?i)\$(Id|Header|Author|Date|DateTime|Change|File|Revision):[^$]*\$', r'$\1$', sys.stdin.read()))
+	EOF
+	cat >ko_smush.py <<-\EOF
+	import re, sys
+	sys.stdout.write(re.sub(r'(?i)\$(Id|Header):[^$]*\$', r'$\1$', sys.stdin.read()))
+	EOF
+}
+
+test_expect_success 'keyword file test' '
+	build_smush &&
+	test_when_finished rm -f k_smush.py ko_smush.py &&
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --dest="$git" //depot@all &&
+	(
+		cd "$git" &&
+
+		# text, ensure unexpanded
+		"$PYTHON_PATH" "$TRASH_DIRECTORY/k_smush.py" <"$cli/k-text-k" >cli-k-text-k-smush &&
+		test_cmp cli-k-text-k-smush k-text-k &&
+		"$PYTHON_PATH" "$TRASH_DIRECTORY/ko_smush.py" <"$cli/k-text-ko" >cli-k-text-ko-smush &&
+		test_cmp cli-k-text-ko-smush k-text-ko &&
+
+		# utf16, even though p4 expands keywords, git-p4 does not
+		# try to undo that
+		test_cmp "$cli/k-utf16-k" k-utf16-k &&
+		test_cmp "$cli/k-utf16-ko" k-utf16-ko
+	)
+'
+
+test_expect_success 'kill p4d' '
+	kill_p4d
+'
+
+test_done
-- 
1.7.7

^ permalink raw reply related

* Re: [PATCH 2/4] git-gui: add smart case search mode in searchbar
From: Andrew Ardill @ 2011-10-16 22:32 UTC (permalink / raw)
  To: Bert Wesarg; +Cc: Pat Thoyts, git
In-Reply-To: <9350c86dc58e6345b237de5af186718d97fdd19b.1318579823.git.bert.wesarg@googlemail.com>

On 14 October 2011 19:14, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> Setting config gui.search.smartcase to true, the search mode in the
> searchbar (from the blame view) is by default case-insensitive. But
> entering an upper case letter into the search field activates the case-
> sensitive search mode.
>
> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> ---
>  lib/search.tcl |   13 ++++++++++++-
>  1 files changed, 12 insertions(+), 1 deletions(-)
>
> diff --git a/lib/search.tcl b/lib/search.tcl
> index ef3486f..461c66d 100644
> --- a/lib/search.tcl
> +++ b/lib/search.tcl
> @@ -7,7 +7,8 @@ field w
>  field ctext
>
>  field searchstring   {}
> -field casesensitive  1
> +field casesensitive
> +field default_casesensitive
>  field searchdirn     -forwards
>
>  field smarktop
> @@ -18,6 +19,12 @@ constructor new {i_w i_text args} {
>        set w      $i_w
>        set ctext  $i_text
>
> +       if {[is_config_true gui.search.smartcase]} {
> +               set default_casesensitive 0
> +       } else {
> +               set default_casesensitive 1
> +       }
> +
>        ${NS}::frame  $w
>        ${NS}::label  $w.l       -text [mc Find:]
>        entry  $w.ent -textvariable ${__this}::searchstring -background lightgreen
> @@ -45,6 +52,7 @@ constructor new {i_w i_text args} {
>  method show {} {
>        if {![visible $this]} {
>                grid $w
> +               set casesensitive $default_casesensitive
>        }
>        focus -force $w.ent
>  }
> @@ -125,6 +133,9 @@ method _incrsearch {} {
>        if {[catch {$ctext index anchor}]} {
>                $ctext mark set anchor [_get_new_anchor $this]
>        }
> +       if {[regexp {[[:upper:]]} $searchstring]} {
> +               set casesensitive 1
> +       }
>        if {$searchstring ne {}} {
>                set here [_do_search $this anchor mlen]
>                if {$here ne {}} {
> --
> 1.7.6.789.gb4599
>

I don't really know tcl so I'm not certain, but it looks like you
never reset the case sensitive flag once it has been set by entering
an upper case letter. If I accidentally enter an upper case letter and
have set smartcase true, I would expect that deleting that character
would turn case sensitivity off again.

Regards,

Andrew Ardill

^ permalink raw reply

* [PATCH 2/2] daemon: report permission denied error to clients
From: Clemens Buchacher @ 2011-10-16 22:11 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <1318803076-4229-1-git-send-email-drizzd@aon.at>

If passed an inaccessible url, git daemon returns the
following error:

 $ git clone git://host/repo
 fatal: remote error: no such repository: /repo

In case of a permission denied error, return the following
instead:

 fatal: remote error: permission denied: /repo

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---
 daemon.c              |   32 +++++++++++++++++++++-----------
 path.c                |   31 +++++++++++++++++++++----------
 t/t5570-git-daemon.sh |    2 +-
 3 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/daemon.c b/daemon.c
index 72fb53a..1442b5b 100644
--- a/daemon.c
+++ b/daemon.c
@@ -109,7 +109,7 @@ static void NORETURN daemon_die(const char *err, va_list params)
 	exit(1);
 }
 
-static char *path_ok(char *directory)
+static int path_ok(char *directory, const char **return_path)
 {
 	static char rpath[PATH_MAX];
 	static char interp_path[PATH_MAX];
@@ -120,13 +120,13 @@ static char *path_ok(char *directory)
 
 	if (daemon_avoid_alias(dir)) {
 		logerror("'%s': aliased", dir);
-		return NULL;
+		return -1;
 	}
 
 	if (*dir == '~') {
 		if (!user_path) {
 			logerror("'%s': User-path not allowed", dir);
-			return NULL;
+			return EACCES;
 		}
 		if (*user_path) {
 			/* Got either "~alice" or "~alice/foo";
@@ -158,7 +158,7 @@ static char *path_ok(char *directory)
 		if (*dir != '/') {
 			/* Allow only absolute */
 			logerror("'%s': Non-absolute path denied (interpolated-path active)", dir);
-			return NULL;
+			return EACCES;
 		}
 
 		strbuf_expand(&expanded_path, interpolated_path,
@@ -173,7 +173,7 @@ static char *path_ok(char *directory)
 		if (*dir != '/') {
 			/* Allow only absolute */
 			logerror("'%s': Non-absolute path denied (base-path active)", dir);
-			return NULL;
+			return EACCES;
 		}
 		snprintf(rpath, PATH_MAX, "%s%s", base_path, dir);
 		dir = rpath;
@@ -190,10 +190,14 @@ static char *path_ok(char *directory)
 	}
 
 	if (!path) {
+		int ret = -1;
+		if (errno == EACCES)
+		       ret = EACCES;
 		logerror("'%s' does not appear to be a git repository", dir);
-		return NULL;
+		return ret;
 	}
 
+	*return_path = path;
 	if ( ok_paths && *ok_paths ) {
 		char **pp;
 		int pathlen = strlen(path);
@@ -211,17 +215,17 @@ static char *path_ok(char *directory)
 			    !memcmp(*pp, path, len) &&
 			    (path[len] == '\0' ||
 			     (!strict_paths && path[len] == '/')))
-				return path;
+				return 0;
 		}
 	}
 	else {
 		/* be backwards compatible */
 		if (!strict_paths)
-			return path;
+			return 0;
 	}
 
 	logerror("'%s': not in whitelist", path);
-	return NULL;		/* Fallthrough. Deny by default */
+	return EACCES;		/* Fallthrough. Deny by default */
 }
 
 typedef int (*daemon_service_fn)(void);
@@ -258,6 +262,7 @@ static int daemon_error(const char *dir, const char *msg)
 
 static int run_service(char *dir, struct daemon_service *service)
 {
+	int err;
 	const char *path;
 	int enabled = service->enabled;
 
@@ -269,8 +274,13 @@ static int run_service(char *dir, struct daemon_service *service)
 		return daemon_error(dir, "service not enabled");
 	}
 
-	if (!(path = path_ok(dir)))
-		return daemon_error(dir, "no such repository");
+	err = path_ok(dir, &path);
+	if (err) {
+		if (err == EACCES)
+			return daemon_error(dir, "permission denied");
+		else
+			return daemon_error(dir, "no such repository");
+	}
 
 	/*
 	 * Security on the cheap.
diff --git a/path.c b/path.c
index 6f3f5d5..227d8d7 100644
--- a/path.c
+++ b/path.c
@@ -288,6 +288,7 @@ char *enter_repo(char *path, int strict)
 	static char used_path[PATH_MAX];
 	static char validated_path[PATH_MAX];
 
+	errno = 0;
 	if (!path)
 		return NULL;
 
@@ -301,12 +302,15 @@ char *enter_repo(char *path, int strict)
 			path[len-1] = 0;
 			len--;
 		}
-		if (PATH_MAX <= len)
+		if (PATH_MAX <= len) {
+			errno = ENAMETOOLONG;
 			return NULL;
+		}
 		if (path[0] == '~') {
 			char *newpath = expand_user_path(path);
 			if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
 				free(newpath);
+				errno = 0;
 				return NULL;
 			}
 			/*
@@ -319,9 +323,10 @@ char *enter_repo(char *path, int strict)
 			strcpy(validated_path, path);
 			path = used_path;
 		}
-		else if (PATH_MAX - 10 < len)
+		else if (PATH_MAX - 10 < len) {
+			errno = ENAMETOOLONG;
 			return NULL;
-		else {
+		} else {
 			path = strcpy(used_path, path);
 			strcpy(validated_path, path);
 		}
@@ -331,23 +336,29 @@ char *enter_repo(char *path, int strict)
 			if (!access(path, F_OK)) {
 				strcat(validated_path, suffix[i]);
 				break;
+			} else if (errno == EACCES) {
+				return NULL;
 			}
 		}
-		if (!suffix[i] || chdir(path))
+		if (!suffix[i])
+			return NULL;
+		if (chdir(path))
 			return NULL;
 		path = validated_path;
 	}
 	else if (chdir(path))
 		return NULL;
 
-	if (access("objects", X_OK) == 0 && access("refs", X_OK) == 0 &&
-	    validate_headref("HEAD") == 0) {
-		set_git_dir(".");
-		check_repository_format();
-		return path;
+	if (access("objects", X_OK) || access("refs", X_OK))
+		return NULL;
+	if (validate_headref("HEAD")) {
+		errno = 0;
+		return NULL;
 	}
 
-	return NULL;
+	set_git_dir(".");
+	check_repository_format();
+	return path;
 }
 
 int set_shared_perm(const char *path, int mode)
diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
index aa5771a..e6482eb 100755
--- a/t/t5570-git-daemon.sh
+++ b/t/t5570-git-daemon.sh
@@ -141,7 +141,7 @@ start_daemon --informative-errors
 
 test_expect_success 'clone non-existent' "test_remote_error    clone nowhere.git 'no such repository'"
 test_expect_success 'push disabled'      "test_remote_error    push  repo.git    'service not enabled'"
-test_expect_success 'read access denied' "test_remote_error -x fetch repo.git    'no such repository'"
+test_expect_success 'read access denied' "test_remote_error -x fetch repo.git    'permission denied'"
 test_expect_success 'not exported'       "test_remote_error -n fetch repo.git    'repository not exported'"
 
 stop_daemon
-- 
1.7.7

^ permalink raw reply related

* [PATCH 1/2] daemon: add tests
From: Clemens Buchacher @ 2011-10-16 22:11 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <20111014211921.GB16429@sigill.intra.peff.net>

The semantics of the git daemon tests are similar to the http
transport tests.  In fact, they are only a slightly modified copy
of t5550, plus the newly added remote error tests.

All daemon tests will be skipped unless the environment variable
GIT_TEST_DAEMON is set.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---

This patch is based on jk/daemon-msgs.

 t/lib-daemon.sh       |   52 +++++++++++++++++
 t/t5570-git-daemon.sh |  148 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 200 insertions(+), 0 deletions(-)
 create mode 100644 t/lib-daemon.sh
 create mode 100755 t/t5570-git-daemon.sh

diff --git a/t/lib-daemon.sh b/t/lib-daemon.sh
new file mode 100644
index 0000000..30a89ea
--- /dev/null
+++ b/t/lib-daemon.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+if test -z "$GIT_TEST_DAEMON"
+then
+	skip_all="Daemon testing disabled (define GIT_TEST_DAEMON to enable)"
+	test_done
+fi
+
+LIB_DAEMON_PORT=${LIB_DAEMON_PORT-'8121'}
+
+DAEMON_PID=
+DAEMON_DOCUMENT_ROOT_PATH="$PWD"/repo
+DAEMON_URL=git://127.0.0.1:$LIB_DAEMON_PORT
+
+start_daemon() {
+	if test -n "$DAEMON_PID"
+	then
+		error "start_daemon already called"
+	fi
+
+	mkdir -p "$DAEMON_DOCUMENT_ROOT_PATH"
+
+	trap 'code=$?; stop_daemon; (exit $code); die' EXIT
+
+	say >&3 "Starting git daemon ..."
+	git daemon --listen=127.0.0.1 --port="$LIB_DAEMON_PORT" \
+		--reuseaddr --verbose \
+		--base-path="$DAEMON_DOCUMENT_ROOT_PATH" \
+		"$@" "$DAEMON_DOCUMENT_ROOT_PATH" \
+		>&3 2>&4 &
+	DAEMON_PID=$!
+}
+
+stop_daemon() {
+	if test -z "$DAEMON_PID"
+	then
+		return
+	fi
+
+	trap 'die' EXIT
+
+	# kill git-daemon child of git
+	say >&3 "Stopping git daemon ..."
+	pkill -P "$DAEMON_PID"
+	wait "$DAEMON_PID"
+	ret=$?
+	if test $ret -ne 143
+	then
+		error "git daemon exited with status: $ret"
+	fi
+	DAEMON_PID=
+}
diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
new file mode 100755
index 0000000..aa5771a
--- /dev/null
+++ b/t/t5570-git-daemon.sh
@@ -0,0 +1,148 @@
+#!/bin/sh
+
+test_description='test fetching over git protocol'
+. ./test-lib.sh
+
+. "$TEST_DIRECTORY"/lib-daemon.sh
+start_daemon
+
+test_expect_success 'setup repository' '
+	echo content >file &&
+	git add file &&
+	git commit -m one
+'
+
+test_expect_success 'create git-accessible bare repository' '
+	mkdir "$DAEMON_DOCUMENT_ROOT_PATH/repo.git" &&
+	(cd "$DAEMON_DOCUMENT_ROOT_PATH/repo.git" &&
+	 git --bare init &&
+	 : >git-daemon-export-ok
+	) &&
+	git remote add public "$DAEMON_DOCUMENT_ROOT_PATH/repo.git" &&
+	git push public master:master
+'
+
+test_expect_success 'clone git repository' '
+	git clone $DAEMON_URL/repo.git clone &&
+	test_cmp file clone/file
+'
+
+test_expect_success 'fetch changes via git protocol' '
+	echo content >>file &&
+	git commit -a -m two &&
+	git push public &&
+	(cd clone && git pull) &&
+	test_cmp file clone/file
+'
+
+test_expect_failure 'remote detects correct HEAD' '
+	git push public master:other &&
+	(cd clone &&
+	 git remote set-head -d origin &&
+	 git remote set-head -a origin &&
+	 git symbolic-ref refs/remotes/origin/HEAD > output &&
+	 echo refs/remotes/origin/master > expect &&
+	 test_cmp expect output
+	)
+'
+
+test_expect_success 'prepare pack objects' '
+	cp -R "$DAEMON_DOCUMENT_ROOT_PATH"/repo.git "$DAEMON_DOCUMENT_ROOT_PATH"/repo_pack.git &&
+	(cd "$DAEMON_DOCUMENT_ROOT_PATH"/repo_pack.git &&
+	 git --bare repack &&
+	 git --bare prune-packed
+	)
+'
+
+test_expect_success 'fetch notices corrupt pack' '
+	cp -R "$DAEMON_DOCUMENT_ROOT_PATH"/repo_pack.git "$DAEMON_DOCUMENT_ROOT_PATH"/repo_bad1.git &&
+	(cd "$DAEMON_DOCUMENT_ROOT_PATH"/repo_bad1.git &&
+	 p=`ls objects/pack/pack-*.pack` &&
+	 chmod u+w $p &&
+	 printf %0256d 0 | dd of=$p bs=256 count=1 seek=1 conv=notrunc
+	) &&
+	mkdir repo_bad1.git &&
+	(cd repo_bad1.git &&
+	 git --bare init &&
+	 test_must_fail git --bare fetch $DAEMON_URL/repo_bad1.git &&
+	 test 0 = `ls objects/pack/pack-*.pack | wc -l`
+	)
+'
+
+test_expect_success 'fetch notices corrupt idx' '
+	cp -R "$DAEMON_DOCUMENT_ROOT_PATH"/repo_pack.git "$DAEMON_DOCUMENT_ROOT_PATH"/repo_bad2.git &&
+	(cd "$DAEMON_DOCUMENT_ROOT_PATH"/repo_bad2.git &&
+	 p=`ls objects/pack/pack-*.idx` &&
+	 chmod u+w $p &&
+	 printf %0256d 0 | dd of=$p bs=256 count=1 seek=1 conv=notrunc
+	) &&
+	mkdir repo_bad2.git &&
+	(cd repo_bad2.git &&
+	 git --bare init &&
+	 test_must_fail git --bare fetch $DAEMON_URL/repo_bad2.git &&
+	 test 0 = `ls objects/pack | wc -l`
+	)
+'
+
+test_remote_error()
+{
+	do_export=YesPlease
+	while test $# -gt 0
+	do
+		case $1 in
+		-x)
+			shift
+			chmod -X "$DAEMON_DOCUMENT_ROOT_PATH/repo.git"
+			;;
+		-n)
+			shift
+			do_export=
+			;;
+		*)
+			break
+		esac
+	done
+
+	if test $# -ne 3
+	then
+		error "invalid number of arguments"
+	fi
+
+	cmd=$1
+	repo=$2
+	msg=$3
+
+	if test -x "$DAEMON_DOCUMENT_ROOT_PATH/$repo"
+	then
+		if test -n "$do_export"
+		then
+			: >"$DAEMON_DOCUMENT_ROOT_PATH/$repo/git-daemon-export-ok"
+		else
+			rm -f "$DAEMON_DOCUMENT_ROOT_PATH/$repo/git-daemon-export-ok"
+		fi
+	fi
+
+	test_must_fail git "$cmd" "$DAEMON_URL/$repo" 2>output &&
+	echo "fatal: remote error: $msg: /$repo" >expect &&
+	test_cmp expect output
+	ret=$?
+	chmod +X "$DAEMON_DOCUMENT_ROOT_PATH/repo.git"
+	(exit $ret)
+}
+
+msg="access denied or repository not exported"
+test_expect_success 'clone non-existent' "test_remote_error    clone nowhere.git '$msg'"
+test_expect_success 'push disabled'      "test_remote_error    push  repo.git    '$msg'"
+test_expect_success 'read access denied' "test_remote_error -x fetch repo.git    '$msg'"
+test_expect_success 'not exported'       "test_remote_error -n fetch repo.git    '$msg'"
+
+stop_daemon
+start_daemon --informative-errors
+
+test_expect_success 'clone non-existent' "test_remote_error    clone nowhere.git 'no such repository'"
+test_expect_success 'push disabled'      "test_remote_error    push  repo.git    'service not enabled'"
+test_expect_success 'read access denied' "test_remote_error -x fetch repo.git    'no such repository'"
+test_expect_success 'not exported'       "test_remote_error -n fetch repo.git    'repository not exported'"
+
+stop_daemon
+test_done
-- 
1.7.7

^ permalink raw reply related

* Re: [BUG] git checkout <branch> allowed with uncommitted changes
From: Holger Hellmuth @ 2011-10-16 22:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: arQon, git
In-Reply-To: <7vy5wkptan.fsf@alter.siamese.dyndns.org>

Am 16.10.2011 22:37, schrieb Junio C Hamano:
> I doubt the full status output is better behaviour. For one thing, you do
> not need full status as by definition branch switching would only have
> local changes as a result (i.e. you will not see "Changes to be committed"
> section).

?? After "git add" on a changed file I see it under "Changes to be 
committed". And branch switching still works (with newest git from 
master branch):

# On branch two
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#       modified:   test1.txt
#
# Changes not staged for commit:
#   (use "git add <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working 
directory)
#
#       modified:   test2.txt
#

Small inconsistency: On the other branch instead of "Changes not staged 
for commit:" the text "Changed but not updated:" is printed, everything 
else is unchanged

^ permalink raw reply

* Re: [PATCH] gitweb: provide a way to customize html headers
From: Jakub Narebski @ 2011-10-16 21:26 UTC (permalink / raw)
  To: Lénaïc Huard; +Cc: git
In-Reply-To: <201110162256.41431.lenaic@lhuard.fr.eu.org>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-16be, Size: 693 bytes --]

Lþÿ\0énaþÿ\0ïc Huard <lenaic@lhuard.fr.eu.org> writes:

> This allows web sites to add some specific html headers to the pages
> generated by gitweb.

What do you need this for?
 
> The new variable $site_htmlheader can be set to a filename the content
> of which will be inserted at the end of the <head> section of each
> page generated by gitweb.

Hmmm... I wonder if a file with html header fragment (which is quite
specific subset of HTML) is a best solution.

> 
> Signed-off-by: Lþÿ\0énaþÿ\0ïc Huard <lenaic@lhuard.fr.eu.org>
> ---
>  gitweb/INSTALL     |    3 +++

Nb. there is patch in flight adding gitweb.conf(5) and gitweb(1)
manpages...

-- 
Jakub Narþÿ\x01\x19bski

^ permalink raw reply

* [PATCH] gitweb: provide a way to customize html headers
From: Lénaïc Huard @ 2011-10-16 20:56 UTC (permalink / raw)
  To: git

This allows web sites to add some specific html headers to the pages
generated by gitweb.

The new variable $site_htmlheader can be set to a filename the content
of which will be inserted at the end of the <head> section of each
page generated by gitweb.

Signed-off-by: Lénaïc Huard <lenaic@lhuard.fr.eu.org>
---
 gitweb/INSTALL     |    3 +++
 gitweb/Makefile    |    2 ++
 gitweb/gitweb.perl |    7 +++++++
 t/gitweb-lib.sh    |    1 +
 4 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/gitweb/INSTALL b/gitweb/INSTALL
index f5efe74..e23eafa 100644
--- a/gitweb/INSTALL
+++ b/gitweb/INSTALL
@@ -130,6 +130,9 @@ You can specify the following configuration variables when building GIT:
    Points to an .html file which is included on the gitweb project
    overview page ('projects_list' view), if it exists.  Relative to
    gitweb.cgi script.  [Default: indextext.html]
+ * GITWEB_SITE_HTMLHEADER
+   Filename of html code to include in the <head> section of each page.
+   Relative to gitweb.cgi script.  [No default]
  * GITWEB_SITE_HEADER
    Filename of html text to include at top of each page.  Relative to
    gitweb.cgi script.  [No default]
diff --git a/gitweb/Makefile b/gitweb/Makefile
index 1c85b5f..ecfb311 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -34,6 +34,7 @@ GITWEB_CSS = static/gitweb.css
 GITWEB_LOGO = static/git-logo.png
 GITWEB_FAVICON = static/git-favicon.png
 GITWEB_JS = static/gitweb.js
+GITWEB_SITE_HTMLHEADER =
 GITWEB_SITE_HEADER =
 GITWEB_SITE_FOOTER =
 HIGHLIGHT_BIN = highlight
@@ -144,6 +145,7 @@ GITWEB_REPLACE = \
        -e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \
        -e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \
        -e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \
+       -e 's|++GITWEB_SITE_HTMLHEADER++|$(GITWEB_SITE_HTMLHEADER)|g' \
        -e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
        -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
        -e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g'
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 85d64b2..63b0115 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -85,6 +85,8 @@ our $home_link_str = "++GITWEB_HOME_LINK_STR++";
 our $site_name = "++GITWEB_SITENAME++"
                  || ($ENV{'SERVER_NAME'} || "Untitled") . " Git";
 
+# filename of html code to include in the <head> section of each page
+our $site_htmlheader = "++GITWEB_SITE_HTMLHEADER++";
 # filename of html text to include at top of each page
 our $site_header = "++GITWEB_SITE_HEADER++";
 # html text to include at home page
@@ -3879,6 +3881,11 @@ EOF
                print "<base href=\"".esc_url($base_url)."\" />\n";
        }
        print_header_links($status);
+
+       if (defined $site_htmlheader && -f $site_htmlheader) {
+               insert_file($site_htmlheader);
+       }
+
        print "</head>\n" .
              "<body>\n";
 
diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index 292753f..cfe5d74 100644
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -16,6 +16,7 @@ our \$projectroot = "$safe_pwd";
 our \$project_maxdepth = 8;
 our \$home_link_str = 'projects';
 our \$site_name = '[localhost]';
+our \$site_htmlheader = '';
 our \$site_header = '';
 our \$site_footer = '';
 our \$home_text = 'indextext.html';
-- 
1.7.7

^ permalink raw reply related

* Re: [BUG] git checkout <branch> allowed with uncommitted changes
From: Junio C Hamano @ 2011-10-16 20:37 UTC (permalink / raw)
  To: arQon; +Cc: git
In-Reply-To: <loom.20111016T201930-426@post.gmane.org>

arQon <arqon@gmx.com> writes:

> ... better *behavior* is a
> clear win either way.

I doubt the full status output is better behaviour. For one thing, you do
not need full status as by definition branch switching would only have
local changes as a result (i.e. you will not see "Changes to be committed"
section).

But if you really do not want to learn how to read "diff --name-status"
output, here is a patch to allow you say "git checkout -v other_branch".
Hopefully it will help you convince yourself why it is not a better
behaviour.

 builtin/checkout.c |   46 +++++++++++++++++++++++++++++++++-------------
 1 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 49a547a..0c21556 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -28,7 +28,7 @@ static const char * const checkout_usage[] = {
 };
 
 struct checkout_opts {
-	int quiet;
+	int verbosity;
 	int merge;
 	int force;
 	int force_detach;
@@ -291,10 +291,10 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 	return errs;
 }
 
-static void show_local_changes(struct object *head, struct diff_options *opts)
+static void show_local_changes_brief(struct object *head, struct diff_options *opts)
 {
 	struct rev_info rev;
-	/* I think we want full paths, even if we're in a subdirectory. */
+
 	init_revisions(&rev, NULL);
 	rev.diffopt.flags = opts->flags;
 	rev.diffopt.output_format |= DIFF_FORMAT_NAME_STATUS;
@@ -304,6 +304,26 @@ static void show_local_changes(struct object *head, struct diff_options *opts)
 	run_diff_index(&rev, 0);
 }
 
+static void show_local_changes_status(void)
+{
+	const char *argv[] = { "status", NULL };
+
+	run_command_v_opt(argv, RUN_GIT_CMD);
+}
+
+static void show_local_changes(struct checkout_opts *opts,
+			       struct object *head,
+			       struct diff_options *diffopts)
+{
+	if (opts->force || opts->verbosity < 0)
+		return;
+
+	if (0 < opts->verbosity)
+		show_local_changes_status();
+	else
+		show_local_changes_brief(head, diffopts);
+}
+
 static void describe_detached_head(const char *msg, struct commit *commit)
 {
 	struct strbuf sb = STRBUF_INIT;
@@ -326,7 +346,7 @@ static int reset_tree(struct tree *tree, struct checkout_opts *o, int worktree)
 	opts.reset = 1;
 	opts.merge = 1;
 	opts.fn = oneway_merge;
-	opts.verbose_update = !o->quiet;
+	opts.verbose_update = (0 <= o->verbosity);
 	opts.src_index = &the_index;
 	opts.dst_index = &the_index;
 	parse_tree(tree);
@@ -403,7 +423,7 @@ static int merge_working_tree(struct checkout_opts *opts,
 		topts.update = 1;
 		topts.merge = 1;
 		topts.gently = opts->merge && old->commit;
-		topts.verbose_update = !opts->quiet;
+		topts.verbose_update = (0 <= opts->verbosity);
 		topts.fn = twoway_merge;
 		topts.dir = xcalloc(1, sizeof(*topts.dir));
 		topts.dir->flags |= DIR_SHOW_IGNORED;
@@ -478,9 +498,6 @@ static int merge_working_tree(struct checkout_opts *opts,
 	    commit_locked_index(lock_file))
 		die(_("unable to write new index file"));
 
-	if (!opts->force && !opts->quiet)
-		show_local_changes(&new->commit->object, &opts->diff_options);
-
 	return 0;
 }
 
@@ -552,14 +569,14 @@ static void update_refs_for_switch(struct checkout_opts *opts,
 	} else if (opts->force_detach || !new->path) {	/* No longer on any branch. */
 		update_ref(msg.buf, "HEAD", new->commit->object.sha1, NULL,
 			   REF_NODEREF, DIE_ON_ERR);
-		if (!opts->quiet) {
+		if (0 <= opts->verbosity) {
 			if (old->path && advice_detached_head)
 				detach_advice(old->path, new->name);
 			describe_detached_head(_("HEAD is now at"), new->commit);
 		}
 	} else if (new->path) {	/* Switch branches. */
 		create_symref("HEAD", new->path, msg.buf);
-		if (!opts->quiet) {
+		if (0 <= opts->verbosity) {
 			if (old->path && !strcmp(new->path, old->path)) {
 				fprintf(stderr, _("Already on '%s'\n"),
 					new->name);
@@ -584,7 +601,7 @@ static void update_refs_for_switch(struct checkout_opts *opts,
 	}
 	remove_branch_state();
 	strbuf_release(&msg);
-	if (!opts->quiet &&
+	if (0 <= opts->verbosity &&
 	    (new->path || (!opts->force_detach && !strcmp(new->name, "HEAD"))))
 		report_tracking(new);
 }
@@ -717,13 +734,16 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
 	if (ret)
 		return ret;
 
-	if (!opts->quiet && !old.path && old.commit && new->commit != old.commit)
+	if (0 <= opts->verbosity && !old.path && old.commit && new->commit != old.commit)
 		orphaned_commit_warning(old.commit);
 
 	update_refs_for_switch(opts, &old, new);
 
 	ret = post_checkout_hook(old.commit, new->commit, 1);
 	free((char *)old.path);
+
+	show_local_changes(opts, &new->commit->object, &opts->diff_options);
+
 	return ret || opts->writeout_error;
 }
 
@@ -906,7 +926,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	int patch_mode = 0;
 	int dwim_new_local_branch = 1;
 	struct option options[] = {
-		OPT__QUIET(&opts.quiet, "suppress progress reporting"),
+		OPT__VERBOSITY(&opts.verbosity),
 		OPT_STRING('b', NULL, &opts.new_branch, "branch",
 			   "create and checkout a new branch"),
 		OPT_STRING('B', NULL, &opts.new_branch_force, "branch",

^ permalink raw reply related

* Re: [BUG] git checkout <branch> allowed with uncommitted changes
From: arQon @ 2011-10-16 18:25 UTC (permalink / raw)
  To: git
In-Reply-To: <20111014095447.GC2856@victor.terreactive.ch>

Victor Engmark <victor.engmark <at> terreactive.ch> writes:
> Very good point. How about by default just running `git status` after a
> successful checkout, and only printing the result if there are any
> changes? That way:
> 1) If no changes are pending, nothing is displayed.
> 2) The user sees a *familiar* style output if anything changed.
> 3) If there's an alias for "status", it would be used.

I'm sold on this. Better documentation for checkout wouldn't hurt regardless,
and I'm still planning on that when I get a chance; but better *behavior* is a
clear win either way. Adding half a page of text to the docs explaining what
each status char means is a hugely-inferior "solution" to simply not having
an aberrant status-ish output in the first place.

^ permalink raw reply

* regression in git-gui since 2c5c66b... Merge branch 'jp/get-ref-dir-unsorted
From: Mark Levedahl @ 2011-10-16 18:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

I have a project organized as a number of nested git modules (not using 
git-submodule), and frequently use git-new-workdir to create the nested 
modules.

Since the above merge-commit, git-gui is confused by this arrangement 
and reports every file in every nested module as being an untracked file 
in the top-level (super) project. Prior to the above merge, git-gui 
properly stops recursing when finding a .git directory. git-gui also 
continues working correctly when the modules are full clones, it just 
doesn't work correctly when the .git directory contains symlinks to the 
real .git directory contents.

git-gui works correctly on either parent of the above merge, just not 
after the merge. As the merge was not clean, I guess Junio gets to 
decide who owns the problem :^).

Note that core git is fine up to current master, it is only git-gui that 
has become confused (e.g., git-status shows the top-level directory of 
each nested module as untracked, but does not list files in the nested 
modules).


Mark

^ permalink raw reply

* Re: [PATCH 2/6] git-p4: handle utf16 filetype properly
From: Junio C Hamano @ 2011-10-16 18:08 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: git, Luke Diamand, Chris Li, Stefano Lattarini
In-Reply-To: <20111016144435.GE22144@arf.padd.com>

Pete Wyckoff <pw@padd.com> writes:

> +build_smush() {
> +	cat >k_smush.py <<-\EOF &&
> +	import re, sys
> +	sys.stdout.write(re.sub(r'(?i)\$(Id|Header|Author|Date|DateTime|Change|File|Revision):[^$]*\$', r'$\1$', sys.stdin.read()))
> +	EOF
> +	cat >ko_smush.py <<-\EOF
> +	import re, sys
> +	sys.stdout.write(re.sub(r'(?i)\$(Id|Header):[^$]*\$', r'$\1$', sys.stdin.read()))
> +	EOF
> +}
> +
> +test_expect_success 'keyword file test' '
> +	build_smush &&
> +	test_when_finished rm -f k_smush.py ko_smush.py &&
> +	test_when_finished cleanup_git &&
> +	"$GITP4" clone --dest="$git" //depot@all &&
> +	(
> +		cd "$git" &&
> +
> +		# text, ensure unexpanded
> +		python "$TRASH_DIRECTORY/k_smush.py" <"$cli/k-text-k" >cli-k-text-k-smush &&
> +		test_cmp cli-k-text-k-smush k-text-k &&
> +		python "$TRASH_DIRECTORY/ko_smush.py" <"$cli/k-text-ko" >cli-k-text-ko-smush &&
> +		test_cmp cli-k-text-ko-smush k-text-ko &&
> +
> +		# utf16, even though p4 expands keywords, git-p4 does not
> +		# try to undo that
> +		test_cmp "$cli/k-utf16-k" k-utf16-k &&
> +		test_cmp "$cli/k-utf16-ko" k-utf16-ko
> +	)
> +'

Shouldn't this pay attention to PYTHON_PATH in any way?

^ permalink raw reply

* Re: Higher-level change review?
From: Jakub Narebski @ 2011-10-16 18:03 UTC (permalink / raw)
  To: Tay Ray Chuan; +Cc: Dave Abrahams, Git Mailing List, magit
In-Reply-To: <CALUzUxpr4FhjJ8OpYcpZOJLZuvveBNzKWd7soY6LQrz0Do1TDg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

[Sending via email, not via GMane; sorry if you got duplications]

Tay Ray Chuan writes:
> On Sun, Oct 16, 2011 at 10:31 PM, Dave Abrahams wrote:
> >
> > I've discovered that Git's diff format is poorly-suited to reviewing the
> > kinds of structural modifications I often deal with, where indentation
> > changes and large parts of documents are reorganized.
> 
> Something off the top of my head:
> 
>   git (diff|show) -w

While -w, --ignore-all-space (and its lesser variant -b, --ignore-space-change)
are nice and good, they cannot deal with code movement.

I have saved somewhere a shell script involving "git blame -w -C -C HEAD^.."
plus some filtering to see what changed beside reordering... but I seem to
have it misplaced.

Found it:

From: Junio C Hamano <gitster-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
http://permalink.gmane.org/gmane.comp.version-control.git/174966
http://thread.gmane.org/gmane.comp.version-control.git/174954/focus=174966
JH>
JH> "git blame" tip of the day. After applying a series like this on a topic
JH> branch, running
JH>
JH>   $ git blame -C master.. -- gitweb/INSTALL | grep -C 3 -e '^[^^]' | less -S
JH>
JH> lets us view the lines without drowning in the bulk of lines that were
JH> merely moved.

HTH
-- 
Jakub Narębski

^ permalink raw reply

* Re: What's cooking in git.git (Oct 2011, #05; Fri, 14)
From: Jeff King @ 2011-10-16 17:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvcrorh49.fsf@alter.siamese.dyndns.org>

On Sun, Oct 16, 2011 at 10:17:10AM -0700, Junio C Hamano wrote:

> >> * jk/pull-rebase-with-work-tree (2011-10-13) 1 commit
> >>  - pull,rebase: handle GIT_WORK_TREE better
> >> 
> >> Looked reasonable.
> >> Will merge to 'next'.
> >
> > I'm not so sure. Didn't you demonstrate that cd_to_toplevel as-is will
> > not actually go to the toplevel if we're outside of the work tree?
> >
> > And changing it is non-trivial, because there may be weird cases that
> > rely on staying there? See my final note in the thread:
> >
> >   http://article.gmane.org/gmane.comp.version-control.git/183519
> 
> Hmm, I might be mistaken, but my impression was that sane people do not do
> so, that the discussion that originated this proposed patch was not such a
> use case, and most importantly that fixing unsane ones is just the matter
> for them to set GIT_WORKING_TREE correctly. So if anything, wouldn't
> getting this in as early as possible to 'master' or at least 'next' help
> catching a flaw in the above logic and possible downside in the real
> world?

Hmm. I thought there were two separate problems:

  1. my analysis was wrong, and "git rev-parse --show-toplevel" did not
     actually show the root of the work tree when we were outside it
     (and therefore cd_to_toplevel did not actually go anywhere)

  2. some people might be outside of the work tree, set GIT_DIR
     explicitly, but not bother setting GIT_WORK_TREE

But I was wrong on (1). If GIT_WORK_TREE is set, we _will_ actually go
to its work tree, which is what we want. If it's not set, then we go
nowhere, and assume the cwd is the work tree. Which is compatible with
current behavior, and makes (2) still work.

So I was thinking there was more problem then there is. I agree we
should let it go to 'next' to shake out any bugs.

Sorry for the noise.

-Peff

^ permalink raw reply

* Re: What's cooking in git.git (Oct 2011, #05; Fri, 14)
From: Junio C Hamano @ 2011-10-16 17:17 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20111016165329.GA14226@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Fri, Oct 14, 2011 at 04:23:21PM -0700, Junio C Hamano wrote:
>
>> * jk/pull-rebase-with-work-tree (2011-10-13) 1 commit
>>  - pull,rebase: handle GIT_WORK_TREE better
>> 
>> Looked reasonable.
>> Will merge to 'next'.
>
> I'm not so sure. Didn't you demonstrate that cd_to_toplevel as-is will
> not actually go to the toplevel if we're outside of the work tree?
>
> And changing it is non-trivial, because there may be weird cases that
> rely on staying there? See my final note in the thread:
>
>   http://article.gmane.org/gmane.comp.version-control.git/183519

Hmm, I might be mistaken, but my impression was that sane people do not do
so, that the discussion that originated this proposed patch was not such a
use case, and most importantly that fixing unsane ones is just the matter
for them to set GIT_WORKING_TREE correctly. So if anything, wouldn't
getting this in as early as possible to 'master' or at least 'next' help
catching a flaw in the above logic and possible downside in the real
world?

>> * jk/daemon-msgs (2011-10-14) 1 commit
>>  - daemon: give friendlier error messages to clients
>> 
>> Will merge to 'next'.
>
> I'm happy to tweak the "access denied" message if other people want. I
> kind of hoped it wouldn't matter, and that most sites would use
> --informative-errors.

I've already updated it with the "not exported" bit from Sitaram.

Thanks.

^ permalink raw reply

* [PATCH] gitweb: add extensions to highlight feature map
From: Lénaïc Huard @ 2011-10-16 16:59 UTC (permalink / raw)
  To: git

added: hpp, m

Signed-off-by: Lénaïc Huard <lenaic@lhuard.fr.eu.org>
---
 gitweb/gitweb.perl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 85d64b2..75e4854 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -262,11 +262,12 @@ our %highlight_ext = (
        # alternate extensions, see /etc/highlight/filetypes.conf
        'h' => 'c',
        map { $_ => 'sh'  } qw(bash zsh ksh),
-       map { $_ => 'cpp' } qw(cxx c++ cc),
+       map { $_ => 'cpp' } qw(cxx c++ cc hpp),
        map { $_ => 'php' } qw(php3 php4 php5 phps),
        map { $_ => 'pl'  } qw(perl pm), # perhaps also 'cgi'
        map { $_ => 'make'} qw(mak mk),
        map { $_ => 'xml' } qw(xhtml html htm),
+       'm' => 'objc',
 );
 
 # You define site-wide feature defaults here; override them with
-- 
1.7.7

^ permalink raw reply related

* Re: What's cooking in git.git (Oct 2011, #05; Fri, 14)
From: Jeff King @ 2011-10-16 16:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd3dzximu.fsf@alter.siamese.dyndns.org>

On Fri, Oct 14, 2011 at 04:23:21PM -0700, Junio C Hamano wrote:

> * jk/pull-rebase-with-work-tree (2011-10-13) 1 commit
>  - pull,rebase: handle GIT_WORK_TREE better
> 
> Looked reasonable.
> Will merge to 'next'.

I'm not so sure. Didn't you demonstrate that cd_to_toplevel as-is will
not actually go to the toplevel if we're outside of the work tree?

And changing it is non-trivial, because there may be weird cases that
rely on staying there? See my final note in the thread:

  http://article.gmane.org/gmane.comp.version-control.git/183519

> * jk/daemon-msgs (2011-10-14) 1 commit
>  - daemon: give friendlier error messages to clients
> 
> Will merge to 'next'.

I'm happy to tweak the "access denied" message if other people want. I
kind of hoped it wouldn't matter, and that most sites would use
--informative-errors.

-Peff

^ permalink raw reply

* Re: [PATCH 2/6] git-p4: handle utf16 filetype properly
From: Pete Wyckoff @ 2011-10-16 15:22 UTC (permalink / raw)
  To: Stefano Lattarini; +Cc: git, Junio C Hamano, Luke Diamand, Chris Li
In-Reply-To: <201110161659.22261.stefano.lattarini@gmail.com>

stefano.lattarini@gmail.com wrote on Sun, 16 Oct 2011 16:59 +0200:
> Hi Pete, and thanks for taking my previous remarks into account.  I have
> one more nit/question though ...
> 
> On Sunday 16 October 2011, Pete Wyckoff wrote:
> > diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
> > new file mode 100755
> > index 0000000..dff0e02
> > --- /dev/null
> > +++ b/t/t9802-git-p4-filetype.sh
> > @@ -0,0 +1,108 @@
> > +#!/bin/sh
> > +
> > + [SNIP]
> > +
> > +		printf "three\nline\ntext" >f-ascii &&
> >
> With this command, the `f-ascii' file won't be newline-terminated.  Is
> this intended, or the result of an oversight?  The same goes for further
> similar usages in the rest f the patch.

That is harmless.  I think an earlier version used to count the
lines, but now it is just random text.  Ditto the other places.
A trailing \n might be prettier, though, for some future debugging
of the tests.  I tested that both with and without \n pass.

		-- Pete

^ permalink raw reply

* Re: Higher-level change review?
From: Tay Ray Chuan @ 2011-10-16 15:05 UTC (permalink / raw)
  To: Dave Abrahams; +Cc: Git Mailing List, magit
In-Reply-To: <m27h450zzc.fsf-NtBv8x4kbP9fRAUK6RR3EeqUGfbH9hYC@public.gmane.org>

On Sun, Oct 16, 2011 at 10:31 PM, Dave Abrahams <dave-xT6NqnoQrPdWk0Htik3J/w@public.gmane.org> wrote:
> I've discovered that Git's diff format is poorly-suited to reviewing the
> kinds of structural modifications I often deal with, where indentation
> changes and large parts of documents are reorganized.

Something off the top of my head:

  git (diff|show) -w

?

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: [PATCH 2/6] git-p4: handle utf16 filetype properly
From: Stefano Lattarini @ 2011-10-16 14:59 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: git, Junio C Hamano, Luke Diamand, Chris Li
In-Reply-To: <20111016144435.GE22144@arf.padd.com>

Hi Pete, and thanks for taking my previous remarks into account.  I have
one more nit/question though ...

On Sunday 16 October 2011, Pete Wyckoff wrote:
> diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
> new file mode 100755
> index 0000000..dff0e02
> --- /dev/null
> +++ b/t/t9802-git-p4-filetype.sh
> @@ -0,0 +1,108 @@
> +#!/bin/sh
> +
> + [SNIP]
> +
> +		printf "three\nline\ntext" >f-ascii &&
>
With this command, the `f-ascii' file won't be newline-terminated.  Is
this intended, or the result of an oversight?  The same goes for further
similar usages in the rest f the patch.

Regards,
  Stefano

^ 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