* Re: [PATCH 2/2 v5] Git.pm: add test suite
From: Jakub Narebski @ 2008-06-19 20:53 UTC (permalink / raw)
To: Lea Wiemann; +Cc: git, Lea Wiemann
In-Reply-To: <1213907569-6393-1-git-send-email-LeWiemann@gmail.com>
Lea Wiemann <lewiemann@gmail.com> writes:
> Changes since v4:
>
> - Use 5.006002 (lowest possible version for Test::More).
[...]
> #!/usr/bin/perl
> use lib (split(/:/, $ENV{GITPERLLIB}));
>
> +use 5.006002; # Test::More was introduced in 5.6.2
Isn't "use Test::More" enough, so this line is not strictly
necessary?
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables
From: Brandon Casey @ 2008-06-19 20:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Mike Hommey, Git Mailing List
In-Reply-To: <7vd4mduv3t.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> A Subshell?
>
> @@ -212,6 +212,7 @@ test_expect_success 'do not fire
> # Must fail due to conflict
> test_must_fail git cherry-pick -n master &&
> echo "editor not started" >.git/result &&
> - test_must_fail GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" git commit &&
> + ( GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" && export GIT_EDITOR &&
> + test_must_fail git commit ) &&
> test "$(cat .git/result)" = "editor not started"
> '
That works and it's better.
-brandon
^ permalink raw reply
* [PATCH 2/2 v5] Git.pm: add test suite
From: Lea Wiemann @ 2008-06-19 20:32 UTC (permalink / raw)
To: git; +Cc: Lea Wiemann
In-Reply-To: <1244a3347b6f15120e57f6b9223a4e2db06479df.1213899000.git.LeWiemann@gmail.com>
Add a shell script (t/t9700-perl-git.sh) that sets up a git repository
and a perl script (t/t9700/test.pl) that runs the actual tests.
Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
---
[Resent with fixed diff to v4 so git-am doesn't get confused. ;-)]
Changes since v4:
- Added missing ampersand (thanks Olivier!).
- Use 5.006002 (lowest possible version for Test::More).
- Use File::Temp instead of the external IO::String.
Tested with Perl 5.6, 5.8, 5.10. Diff against v4 of this patch:
index 592d79a..b2fb9ec 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -25,7 +25,7 @@ test_expect_success \
git-config --add color.test.slot1 green &&
git-config --add test.string value &&
git-config --add test.dupstring value1 &&
- git-config --add test.dupstring value2 &
+ git-config --add test.dupstring value2 &&
git-config --add test.booltrue true &&
git-config --add test.boolfalse no &&
git-config --add test.boolother other &&
diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index 8318fec..4d23125 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -1,6 +1,7 @@
#!/usr/bin/perl
use lib (split(/:/, $ENV{GITPERLLIB}));
+use 5.006002; # Test::More was introduced in 5.6.2
use warnings;
use strict;
@@ -9,7 +10,6 @@ use Test::More qw(no_plan);
use Cwd;
use File::Basename;
use File::Temp;
-use IO::String;
BEGIN { use_ok('Git') }
@@ -69,20 +69,21 @@ is($r->ident_person("Name", "email", "123 +0000"), "Name <email>",
# objects and hashes
ok(our $file1hash = $r->command_oneline('rev-parse', "HEAD:file1"), "(get file hash)");
-our $iostring = IO::String->new;
-is($r->cat_blob($file1hash, $iostring), 15, "cat_blob: size");
-is(${$iostring->string_ref}, "changed file 1\n", "cat_blob: data");
-our $tmpfile = File::Temp->new();
-print $tmpfile ${$iostring->string_ref};
+our $tmpfile = File::Temp->new;
+is($r->cat_blob($file1hash, $tmpfile), 15, "cat_blob: size");
+our $blobcontents;
+{ local $/; seek $tmpfile, 0, 0; $blobcontents = <$tmpfile>; }
+is($blobcontents, "changed file 1\n", "cat_blob: data");
+seek $tmpfile, 0, 0;
is(Git::hash_object("blob", $tmpfile), $file1hash, "hash_object: roundtrip");
$tmpfile = File::Temp->new();
print $tmpfile my $test_text = "test blob, to be inserted\n";
-$tmpfile->close;
like(our $newhash = $r->hash_and_insert_object($tmpfile), qr/[0-9a-fA-F]{40}/,
"hash_and_insert_object: returns hash");
-$iostring = IO::String->new;
-is($r->cat_blob($newhash, $iostring), length $test_text, "cat_blob: roundtrip size");
-is(${$iostring->string_ref}, $test_text, "cat_blob: roundtrip data");
+$tmpfile = File::Temp->new;
+is($r->cat_blob($newhash, $tmpfile), length $test_text, "cat_blob: roundtrip size");
+{ local $/; seek $tmpfile, 0, 0; $blobcontents = <$tmpfile>; }
+is($blobcontents, $test_text, "cat_blob: roundtrip data");
# paths
is($r->repo_path, "./.git", "repo_path");
t/t9700-perl-git.sh | 39 ++++++++++++++++++++
t/t9700/test.pl | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 139 insertions(+), 0 deletions(-)
create mode 100755 t/t9700-perl-git.sh
create mode 100755 t/t9700/test.pl
diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
new file mode 100755
index 0000000..b2fb9ec
--- /dev/null
+++ b/t/t9700-perl-git.sh
@@ -0,0 +1,39 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Lea Wiemann
+#
+
+test_description='perl interface (Git.pm)'
+. ./test-lib.sh
+
+# set up test repository
+
+test_expect_success \
+ 'set up test repository' \
+ 'echo "test file 1" > file1 &&
+ echo "test file 2" > file2 &&
+ mkdir directory1 &&
+ echo "in directory1" >> directory1/file &&
+ mkdir directory2 &&
+ echo "in directory2" >> directory2/file &&
+ git add . &&
+ git commit -m "first commit" &&
+
+ echo "changed file 1" > file1 &&
+ git commit -a -m "second commit" &&
+
+ git-config --add color.test.slot1 green &&
+ git-config --add test.string value &&
+ git-config --add test.dupstring value1 &&
+ git-config --add test.dupstring value2 &&
+ git-config --add test.booltrue true &&
+ git-config --add test.boolfalse no &&
+ git-config --add test.boolother other &&
+ git-config --add test.int 2k
+ '
+
+test_external_without_stderr \
+ 'Perl API' \
+ perl ../t9700/test.pl
+
+test_done
diff --git a/t/t9700/test.pl b/t/t9700/test.pl
new file mode 100755
index 0000000..4d23125
--- /dev/null
+++ b/t/t9700/test.pl
@@ -0,0 +1,100 @@
+#!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+use 5.006002;
+use warnings;
+use strict;
+
+use Test::More qw(no_plan);
+
+use Cwd;
+use File::Basename;
+use File::Temp;
+
+BEGIN { use_ok('Git') }
+
+# set up
+our $repo_dir = "trash directory";
+our $abs_repo_dir = Cwd->cwd;
+die "this must be run by calling the t/t97* shell script(s)\n"
+ if basename(Cwd->cwd) ne $repo_dir;
+ok(our $r = Git->repository(Directory => "."), "open repository");
+
+# config
+is($r->config("test.string"), "value", "config scalar: string");
+is_deeply([$r->config("test.dupstring")], ["value1", "value2"],
+ "config array: string");
+is($r->config("test.nonexistent"), undef, "config scalar: nonexistent");
+is_deeply([$r->config("test.nonexistent")], [], "config array: nonexistent");
+is($r->config_int("test.int"), 2048, "config_int: integer");
+is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent");
+ok($r->config_bool("test.booltrue"), "config_bool: true");
+ok(!$r->config_bool("test.boolfalse"), "config_bool: false");
+our $ansi_green = "\x1b[32m";
+is($r->get_color("color.test.slot1", "red"), $ansi_green, "get_color");
+# Cannot test $r->get_colorbool("color.foo")) because we do not
+# control whether our STDOUT is a terminal.
+
+# Failure cases for config:
+# Save and restore STDERR; we will probably extract this into a
+# "dies_ok" method and possibly move the STDERR handling to Git.pm.
+open our $tmpstderr, ">&", STDERR or die "cannot save STDERR"; close STDERR;
+eval { $r->config("test.dupstring") };
+ok($@, "config: duplicate entry in scalar context fails");
+eval { $r->config_bool("test.boolother") };
+ok($@, "config_bool: non-boolean values fail");
+open STDERR, ">&", $tmpstderr or die "cannot restore STDERR";
+
+# ident
+like($r->ident("aUthor"), qr/^A U Thor <author\@example.com> [0-9]+ \+0000$/,
+ "ident scalar: author (type)");
+like($r->ident("cOmmitter"), qr/^C O Mitter <committer\@example.com> [0-9]+ \+0000$/,
+ "ident scalar: committer (type)");
+is($r->ident("invalid"), "invalid", "ident scalar: invalid ident string (no parsing)");
+my ($name, $email, $time_tz) = $r->ident('author');
+is_deeply([$name, $email], ["A U Thor", "author\@example.com"],
+ "ident array: author");
+like($time_tz, qr/[0-9]+ \+0000/, "ident array: author");
+is_deeply([$r->ident("Name <email> 123 +0000")], ["Name", "email", "123 +0000"],
+ "ident array: ident string");
+is_deeply([$r->ident("invalid")], [], "ident array: invalid ident string");
+
+# ident_person
+is($r->ident_person("aUthor"), "A U Thor <author\@example.com>",
+ "ident_person: author (type)");
+is($r->ident_person("Name <email> 123 +0000"), "Name <email>",
+ "ident_person: ident string");
+is($r->ident_person("Name", "email", "123 +0000"), "Name <email>",
+ "ident_person: array");
+
+# objects and hashes
+ok(our $file1hash = $r->command_oneline('rev-parse', "HEAD:file1"), "(get file hash)");
+our $tmpfile = File::Temp->new;
+is($r->cat_blob($file1hash, $tmpfile), 15, "cat_blob: size");
+our $blobcontents;
+{ local $/; seek $tmpfile, 0, 0; $blobcontents = <$tmpfile>; }
+is($blobcontents, "changed file 1\n", "cat_blob: data");
+seek $tmpfile, 0, 0;
+is(Git::hash_object("blob", $tmpfile), $file1hash, "hash_object: roundtrip");
+$tmpfile = File::Temp->new();
+print $tmpfile my $test_text = "test blob, to be inserted\n";
+like(our $newhash = $r->hash_and_insert_object($tmpfile), qr/[0-9a-fA-F]{40}/,
+ "hash_and_insert_object: returns hash");
+$tmpfile = File::Temp->new;
+is($r->cat_blob($newhash, $tmpfile), length $test_text, "cat_blob: roundtrip size");
+{ local $/; seek $tmpfile, 0, 0; $blobcontents = <$tmpfile>; }
+is($blobcontents, $test_text, "cat_blob: roundtrip data");
+
+# paths
+is($r->repo_path, "./.git", "repo_path");
+is($r->wc_path, $abs_repo_dir . "/", "wc_path");
+is($r->wc_subdir, "", "wc_subdir initial");
+$r->wc_chdir("directory1");
+is($r->wc_subdir, "directory1", "wc_subdir after wc_chdir");
+TODO: {
+ local $TODO = "commands do not work after wc_chdir";
+ # Failure output is active even in non-verbose mode and thus
+ # annoying. Hence we skip these tests as long as they fail.
+ todo_skip 'config after wc_chdir', 1;
+ is($r->config("color.string"), "value", "config after wc_chdir");
+}
--
1.5.6.149.g06c04.dirty
^ permalink raw reply related
* Re: Including branch info in git format-patch
From: Jeff King @ 2008-06-19 20:28 UTC (permalink / raw)
To: Mukund Sivaraman; +Cc: git
In-Reply-To: <20080619154251.GA16475@jurassic>
On Thu, Jun 19, 2008 at 09:12:52PM +0530, Mukund Sivaraman wrote:
> When i use "git format-patch", it doesn't seem to include the branch (or
> remote) name in the email it creates. So a reader of this mail may not
> know what branch to apply it on to test it. Aside from adding in branch
> information manually in the body of the message, is there any other
> existing way to get git format-patch to include it?
No, there isn't a way to do it automatically. On this mailing list,
patches are assumed to be on top of "master" unless explicitly indicated
otherwise. And we usually just mention it manually.
You could potentially add a config option to put the branch name inside
the '[PATCH]' text. This text is generally stripped away before
applying, so it would still free up the receiver to apply on whatever
branch they wanted. I don't think it would make sense for git
development, since we typically use topic branches, so keeping it
configurable would make sense.
On the receiving end, the applying party could set up their MUA to split
patches based on the subject, and then apply them as appropriate (this
very much depends on their workflow for applying patches). You could
probably even do a pre-applypatch hook as a safety valve to make sure
things were being applied in the right place.
-Peff
^ permalink raw reply
* [PATCH v3] gitweb: standarize HTTP status codes
From: Lea Wiemann @ 2008-06-19 20:25 UTC (permalink / raw)
To: git; +Cc: Lea Wiemann
In-Reply-To: <1213905801-2811-1-git-send-email-LeWiemann@gmail.com>
Many error status codes simply default to 403 Forbidden, which is not
correct in most cases. This patch makes gitweb return semantically
correct status codes.
For convenience the die_error function now only takes the status code
without reason as first parameter (e.g. 404 instead of "404 Not
Found"), and it now defaults to 500 (Internal Server Error), even
though the default is not used anywhere.
Also documented status code conventions in die_error.
Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
---
[Resent with fixed diff to v2 so git-am doesn't get confused. ;-)]
Changes since v2: die_error now adds the reason strings defined by RFC
2616 to the HTTP status code; incorporated Jakub's other suggestions.
Diff to v2 follows.
I didn't use the HTTP_NOT_FOUND etc. suggestion because I found it too
verbose and obtrusive.
Just a friendly reminder, please remember that discussing fairly
trivial changes in-depth might be not a good use of all participants'
time; IOW, sending 340 lines of email about HTTP status codes that
don't *really* matter is probably overkill. (In case you're
interested, the reason why I wrote this patch in the first place is in
order to be able to tell unforeseen errors from expected errors in the
test suite.) I recommend you read http://bikeshed.com/ if you haven't
done so.
Also, increasing the time between patch submission and acceptance
causes added overhead (effort) by itself (i.e. *in addition* to the
time spend on discussing and resending patches); that's because you
cannot base anything on the code without risking merge conflicts.
Please think of that when replying to patches. ;-)
By the way, when you want to make trivial changes, it may be easier
for everyone if you simply implement them and send a follow-up patch
(and it might actually be just as quick as suggesting the changes!).
In your follow-up patch you could provide a diff to the previous
version, and comment on the changes you made (or send it without
comments, even -- oftentimes they're obvious).
Anyways, I hope everyone is happy with this version of the patch.
Diff to v2:
index 3ca45fd..3bc8f69 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2687,9 +2687,11 @@ sub die_error {
my $status = shift || 500;
my $error = shift || "Internal server error";
- # Use a generic "Error" reason, e.g. "404 Error" instead of
- # "404 Not Found". This is permissible by RFC 2616.
- git_header_html("$status Error");
+ my %http_responses = (400 => '400 Bad Request',
+ 403 => '403 Forbidden',
+ 404 => '404 Not Found',
+ 500 => '500 Internal Server Error');
+ git_header_html($http_responses{$status});
print <<EOF;
<div class="page_body">
<br /><br />
@@ -4131,11 +4133,11 @@ sub git_blame {
my $ftype;
gitweb_check_feature('blame')
- or die_error(403, "Blame not allowed");
+ or die_error(403, "Blame view not allowed");
die_error(400, "No file name given") unless $file_name;
$hash_base ||= git_get_head_hash($project);
- die_error(400, "Couldn't find base commit") unless ($hash_base);
+ die_error(404, "Couldn't find base commit") unless ($hash_base);
my %co = parse_commit($hash_base)
or die_error(404, "Commit not found");
if (!defined $hash) {
@@ -4403,7 +4405,7 @@ sub git_tree {
open my $fd, "-|", git_cmd(), "ls-tree", '-z', $hash
or die_error(500, "Open git-ls-tree failed");
my @entries = map { chomp; $_ } <$fd>;
- close $fd or die_error(500, "Reading tree failed");
+ close $fd or die_error(404, "Reading tree failed");
$/ = "\n";
my $refs = git_get_references();
@@ -4641,7 +4643,7 @@ sub git_commit {
$hash, "--"
or die_error(500, "Open git-diff-tree failed");
@difftree = map { chomp; $_ } <$fd>;
- close $fd or die_error(500, "Reading git-diff-tree failed");
+ close $fd or die_error(404, "Reading git-diff-tree failed");
# non-textual hash id's can be cached
my $expires;
@@ -4788,7 +4790,7 @@ sub git_blobdiff {
or die_error(500, "Open git-diff-tree failed");
@difftree = map { chomp; $_ } <$fd>;
close $fd
- or die_error(500, "Reading git-diff-tree failed");
+ or die_error(404, "Reading git-diff-tree failed");
@difftree
or die_error(404, "Blob diff not found");
@@ -4806,7 +4808,7 @@ sub git_blobdiff {
grep { /^:[0-7]{6} [0-7]{6} [0-9a-fA-F]{40} $hash/ }
map { chomp; $_ } <$fd>;
close $fd
- or die_error(500, "Reading git-diff-tree failed");
+ or die_error(404, "Reading git-diff-tree failed");
@difftree
or die_error(404, "Blob diff not found");
gitweb/gitweb.perl | 211 ++++++++++++++++++++++++++--------------------------
1 files changed, 105 insertions(+), 106 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3a7adae..3bc8f69 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -386,7 +386,7 @@ $projects_list ||= $projectroot;
our $action = $cgi->param('a');
if (defined $action) {
if ($action =~ m/[^0-9a-zA-Z\.\-_]/) {
- die_error(undef, "Invalid action parameter");
+ die_error(400, "Invalid action parameter");
}
}
@@ -399,21 +399,21 @@ if (defined $project) {
($export_ok && !(-e "$projectroot/$project/$export_ok")) ||
($strict_export && !project_in_list($project))) {
undef $project;
- die_error(undef, "No such project");
+ die_error(404, "No such project");
}
}
our $file_name = $cgi->param('f');
if (defined $file_name) {
if (!validate_pathname($file_name)) {
- die_error(undef, "Invalid file parameter");
+ die_error(400, "Invalid file parameter");
}
}
our $file_parent = $cgi->param('fp');
if (defined $file_parent) {
if (!validate_pathname($file_parent)) {
- die_error(undef, "Invalid file parent parameter");
+ die_error(400, "Invalid file parent parameter");
}
}
@@ -421,21 +421,21 @@ if (defined $file_parent) {
our $hash = $cgi->param('h');
if (defined $hash) {
if (!validate_refname($hash)) {
- die_error(undef, "Invalid hash parameter");
+ die_error(400, "Invalid hash parameter");
}
}
our $hash_parent = $cgi->param('hp');
if (defined $hash_parent) {
if (!validate_refname($hash_parent)) {
- die_error(undef, "Invalid hash parent parameter");
+ die_error(400, "Invalid hash parent parameter");
}
}
our $hash_base = $cgi->param('hb');
if (defined $hash_base) {
if (!validate_refname($hash_base)) {
- die_error(undef, "Invalid hash base parameter");
+ die_error(400, "Invalid hash base parameter");
}
}
@@ -447,10 +447,10 @@ our @extra_options = $cgi->param('opt');
if (defined @extra_options) {
foreach my $opt (@extra_options) {
if (not exists $allowed_options{$opt}) {
- die_error(undef, "Invalid option parameter");
+ die_error(400, "Invalid option parameter");
}
if (not grep(/^$action$/, @{$allowed_options{$opt}})) {
- die_error(undef, "Invalid option parameter for this action");
+ die_error(400, "Invalid option parameter for this action");
}
}
}
@@ -458,7 +458,7 @@ if (defined @extra_options) {
our $hash_parent_base = $cgi->param('hpb');
if (defined $hash_parent_base) {
if (!validate_refname($hash_parent_base)) {
- die_error(undef, "Invalid hash parent base parameter");
+ die_error(400, "Invalid hash parent base parameter");
}
}
@@ -466,14 +466,14 @@ if (defined $hash_parent_base) {
our $page = $cgi->param('pg');
if (defined $page) {
if ($page =~ m/[^0-9]/) {
- die_error(undef, "Invalid page parameter");
+ die_error(400, "Invalid page parameter");
}
}
our $searchtype = $cgi->param('st');
if (defined $searchtype) {
if ($searchtype =~ m/[^a-z]/) {
- die_error(undef, "Invalid searchtype parameter");
+ die_error(400, "Invalid searchtype parameter");
}
}
@@ -483,7 +483,7 @@ our $searchtext = $cgi->param('s');
our $search_regexp;
if (defined $searchtext) {
if (length($searchtext) < 2) {
- die_error(undef, "At least two characters are required for search parameter");
+ die_error(403, "At least two characters are required for search parameter");
}
$search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext;
}
@@ -580,11 +580,11 @@ if (!defined $action) {
}
}
if (!defined($actions{$action})) {
- die_error(undef, "Unknown action");
+ die_error(400, "Unknown action");
}
if ($action !~ m/^(opml|project_list|project_index)$/ &&
!$project) {
- die_error(undef, "Project needed");
+ die_error(400, "Project needed");
}
$actions{$action}->();
exit;
@@ -1665,7 +1665,7 @@ sub git_get_hash_by_path {
$path =~ s,/+$,,;
open my $fd, "-|", git_cmd(), "ls-tree", $base, "--", $path
- or die_error(undef, "Open git-ls-tree failed");
+ or die_error(500, "Open git-ls-tree failed");
my $line = <$fd>;
close $fd or return undef;
@@ -2127,7 +2127,7 @@ sub parse_commit {
"--max-count=1",
$commit_id,
"--",
- or die_error(undef, "Open git-rev-list failed");
+ or die_error(500, "Open git-rev-list failed");
%co = parse_commit_text(<$fd>, 1);
close $fd;
@@ -2152,7 +2152,7 @@ sub parse_commits {
$commit_id,
"--",
($filename ? ($filename) : ())
- or die_error(undef, "Open git-rev-list failed");
+ or die_error(500, "Open git-rev-list failed");
while (my $line = <$fd>) {
my %co = parse_commit_text($line);
push @cos, \%co;
@@ -2672,11 +2672,26 @@ sub git_footer_html {
"</html>";
}
+# die_error(<http_status_code>, <error_message>)
+# Example: die_error(404, 'Hash not found')
+# By convention, use the following status codes (as defined in RFC 2616):
+# 400: Invalid or missing CGI parameters, or
+# requested object exists but has wrong type.
+# 403: Requested feature (like "pickaxe" or "snapshot") not enabled on
+# this server or project.
+# 404: Requested object/revision/project doesn't exist.
+# 500: The server isn't configured properly, or
+# an internal error occurred (e.g. failed assertions caused by bugs), or
+# an unknown error occurred (e.g. the git binary died unexpectedly).
sub die_error {
- my $status = shift || "403 Forbidden";
- my $error = shift || "Malformed query, file missing or permission denied";
-
- git_header_html($status);
+ my $status = shift || 500;
+ my $error = shift || "Internal server error";
+
+ my %http_responses = (400 => '400 Bad Request',
+ 403 => '403 Forbidden',
+ 404 => '404 Not Found',
+ 500 => '500 Internal Server Error');
+ git_header_html($http_responses{$status});
print <<EOF;
<div class="page_body">
<br /><br />
@@ -3924,12 +3939,12 @@ sub git_search_grep_body {
sub git_project_list {
my $order = $cgi->param('o');
if (defined $order && $order !~ m/none|project|descr|owner|age/) {
- die_error(undef, "Unknown order parameter");
+ die_error(400, "Unknown order parameter");
}
my @list = git_get_projects_list();
if (!@list) {
- die_error(undef, "No projects found");
+ die_error(404, "No projects found");
}
git_header_html();
@@ -3947,12 +3962,12 @@ sub git_project_list {
sub git_forks {
my $order = $cgi->param('o');
if (defined $order && $order !~ m/none|project|descr|owner|age/) {
- die_error(undef, "Unknown order parameter");
+ die_error(400, "Unknown order parameter");
}
my @list = git_get_projects_list($project);
if (!@list) {
- die_error(undef, "No forks found");
+ die_error(404, "No forks found");
}
git_header_html();
@@ -4081,7 +4096,7 @@ sub git_tag {
my %tag = parse_tag($hash);
if (! %tag) {
- die_error(undef, "Unknown tag object");
+ die_error(404, "Unknown tag object");
}
git_print_header_div('commit', esc_html($tag{'name'}), $hash);
@@ -4117,26 +4132,25 @@ sub git_blame {
my $fd;
my $ftype;
- my ($have_blame) = gitweb_check_feature('blame');
- if (!$have_blame) {
- die_error('403 Permission denied', "Permission denied");
- }
- die_error('404 Not Found', "File name not defined") if (!$file_name);
+ gitweb_check_feature('blame')
+ or die_error(403, "Blame view not allowed");
+
+ die_error(400, "No file name given") unless $file_name;
$hash_base ||= git_get_head_hash($project);
- die_error(undef, "Couldn't find base commit") unless ($hash_base);
+ die_error(404, "Couldn't find base commit") unless ($hash_base);
my %co = parse_commit($hash_base)
- or die_error(undef, "Reading commit failed");
+ or die_error(404, "Commit not found");
if (!defined $hash) {
$hash = git_get_hash_by_path($hash_base, $file_name, "blob")
- or die_error(undef, "Error looking up file");
+ or die_error(404, "Error looking up file");
}
$ftype = git_get_type($hash);
if ($ftype !~ "blob") {
- die_error('400 Bad Request', "Object is not a blob");
+ die_error(400, "Object is not a blob");
}
open ($fd, "-|", git_cmd(), "blame", '-p', '--',
$file_name, $hash_base)
- or die_error(undef, "Open git-blame failed");
+ or die_error(500, "Open git-blame failed");
git_header_html();
my $formats_nav =
$cgi->a({-href => href(action=>"blob", -replay=>1)},
@@ -4198,7 +4212,7 @@ HTML
print "</td>\n";
}
open (my $dd, "-|", git_cmd(), "rev-parse", "$full_rev^")
- or die_error(undef, "Open git-rev-parse failed");
+ or die_error(500, "Open git-rev-parse failed");
my $parent_commit = <$dd>;
close $dd;
chomp($parent_commit);
@@ -4255,9 +4269,9 @@ sub git_blob_plain {
if (defined $file_name) {
my $base = $hash_base || git_get_head_hash($project);
$hash = git_get_hash_by_path($base, $file_name, "blob")
- or die_error(undef, "Error lookup file");
+ or die_error(404, "Cannot find file");
} else {
- die_error(undef, "No file name defined");
+ die_error(400, "No file name defined");
}
} elsif ($hash =~ m/^[0-9a-fA-F]{40}$/) {
# blobs defined by non-textual hash id's can be cached
@@ -4265,7 +4279,7 @@ sub git_blob_plain {
}
open my $fd, "-|", git_cmd(), "cat-file", "blob", $hash
- or die_error(undef, "Open git-cat-file blob '$hash' failed");
+ or die_error(500, "Open git-cat-file blob '$hash' failed");
# content-type (can include charset)
$type = blob_contenttype($fd, $file_name, $type);
@@ -4297,9 +4311,9 @@ sub git_blob {
if (defined $file_name) {
my $base = $hash_base || git_get_head_hash($project);
$hash = git_get_hash_by_path($base, $file_name, "blob")
- or die_error(undef, "Error lookup file");
+ or die_error(404, "Cannot find file");
} else {
- die_error(undef, "No file name defined");
+ die_error(400, "No file name defined");
}
} elsif ($hash =~ m/^[0-9a-fA-F]{40}$/) {
# blobs defined by non-textual hash id's can be cached
@@ -4308,7 +4322,7 @@ sub git_blob {
my ($have_blame) = gitweb_check_feature('blame');
open my $fd, "-|", git_cmd(), "cat-file", "blob", $hash
- or die_error(undef, "Couldn't cat $file_name, $hash");
+ or die_error(500, "Couldn't cat $file_name, $hash");
my $mimetype = blob_mimetype($fd, $file_name);
if ($mimetype !~ m!^(?:text/|image/(?:gif|png|jpeg)$)! && -B $fd) {
close $fd;
@@ -4389,9 +4403,9 @@ sub git_tree {
}
$/ = "\0";
open my $fd, "-|", git_cmd(), "ls-tree", '-z', $hash
- or die_error(undef, "Open git-ls-tree failed");
+ or die_error(500, "Open git-ls-tree failed");
my @entries = map { chomp; $_ } <$fd>;
- close $fd or die_error(undef, "Reading tree failed");
+ close $fd or die_error(404, "Reading tree failed");
$/ = "\n";
my $refs = git_get_references();
@@ -4481,16 +4495,16 @@ sub git_snapshot {
my $format = $cgi->param('sf');
if (!@supported_fmts) {
- die_error('403 Permission denied', "Permission denied");
+ die_error(403, "Snapshots not allowed");
}
# default to first supported snapshot format
$format ||= $supported_fmts[0];
if ($format !~ m/^[a-z0-9]+$/) {
- die_error(undef, "Invalid snapshot format parameter");
+ die_error(400, "Invalid snapshot format parameter");
} elsif (!exists($known_snapshot_formats{$format})) {
- die_error(undef, "Unknown snapshot format");
+ die_error(400, "Unknown snapshot format");
} elsif (!grep($_ eq $format, @supported_fmts)) {
- die_error(undef, "Unsupported snapshot format");
+ die_error(403, "Unsupported snapshot format");
}
if (!defined $hash) {
@@ -4518,7 +4532,7 @@ sub git_snapshot {
-status => '200 OK');
open my $fd, "-|", $cmd
- or die_error(undef, "Execute git-archive failed");
+ or die_error(500, "Execute git-archive failed");
binmode STDOUT, ':raw';
print <$fd>;
binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi
@@ -4586,10 +4600,8 @@ sub git_log {
sub git_commit {
$hash ||= $hash_base || "HEAD";
- my %co = parse_commit($hash);
- if (!%co) {
- die_error(undef, "Unknown commit object");
- }
+ my %co = parse_commit($hash)
+ or die_error(404, "Unknown commit object");
my %ad = parse_date($co{'author_epoch'}, $co{'author_tz'});
my %cd = parse_date($co{'committer_epoch'}, $co{'committer_tz'});
@@ -4629,9 +4641,9 @@ sub git_commit {
@diff_opts,
(@$parents <= 1 ? $parent : '-c'),
$hash, "--"
- or die_error(undef, "Open git-diff-tree failed");
+ or die_error(500, "Open git-diff-tree failed");
@difftree = map { chomp; $_ } <$fd>;
- close $fd or die_error(undef, "Reading git-diff-tree failed");
+ close $fd or die_error(404, "Reading git-diff-tree failed");
# non-textual hash id's can be cached
my $expires;
@@ -4724,33 +4736,33 @@ sub git_object {
open my $fd, "-|", quote_command(
git_cmd(), 'cat-file', '-t', $object_id) . ' 2> /dev/null'
- or die_error('404 Not Found', "Object does not exist");
+ or die_error(404, "Object does not exist");
$type = <$fd>;
chomp $type;
close $fd
- or die_error('404 Not Found', "Object does not exist");
+ or die_error(404, "Object does not exist");
# - hash_base and file_name
} elsif ($hash_base && defined $file_name) {
$file_name =~ s,/+$,,;
system(git_cmd(), "cat-file", '-e', $hash_base) == 0
- or die_error('404 Not Found', "Base object does not exist");
+ or die_error(404, "Base object does not exist");
# here errors should not hapen
open my $fd, "-|", git_cmd(), "ls-tree", $hash_base, "--", $file_name
- or die_error(undef, "Open git-ls-tree failed");
+ or die_error(500, "Open git-ls-tree failed");
my $line = <$fd>;
close $fd;
#'100644 blob 0fa3f3a66fb6a137f6ec2c19351ed4d807070ffa panic.c'
unless ($line && $line =~ m/^([0-9]+) (.+) ([0-9a-fA-F]{40})\t/) {
- die_error('404 Not Found', "File or directory for given base does not exist");
+ die_error(404, "File or directory for given base does not exist");
}
$type = $2;
$hash = $3;
} else {
- die_error('404 Not Found', "Not enough information to find object");
+ die_error(400, "Not enough information to find object");
}
print $cgi->redirect(-uri => href(action=>$type, -full=>1,
@@ -4775,12 +4787,12 @@ sub git_blobdiff {
open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
$hash_parent_base, $hash_base,
"--", (defined $file_parent ? $file_parent : ()), $file_name
- or die_error(undef, "Open git-diff-tree failed");
+ or die_error(500, "Open git-diff-tree failed");
@difftree = map { chomp; $_ } <$fd>;
close $fd
- or die_error(undef, "Reading git-diff-tree failed");
+ or die_error(404, "Reading git-diff-tree failed");
@difftree
- or die_error('404 Not Found', "Blob diff not found");
+ or die_error(404, "Blob diff not found");
} elsif (defined $hash &&
$hash =~ /[0-9a-fA-F]{40}/) {
@@ -4789,23 +4801,23 @@ sub git_blobdiff {
# read filtered raw output
open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
$hash_parent_base, $hash_base, "--"
- or die_error(undef, "Open git-diff-tree failed");
+ or die_error(500, "Open git-diff-tree failed");
@difftree =
# ':100644 100644 03b21826... 3b93d5e7... M ls-files.c'
# $hash == to_id
grep { /^:[0-7]{6} [0-7]{6} [0-9a-fA-F]{40} $hash/ }
map { chomp; $_ } <$fd>;
close $fd
- or die_error(undef, "Reading git-diff-tree failed");
+ or die_error(404, "Reading git-diff-tree failed");
@difftree
- or die_error('404 Not Found', "Blob diff not found");
+ or die_error(404, "Blob diff not found");
} else {
- die_error('404 Not Found', "Missing one of the blob diff parameters");
+ die_error(400, "Missing one of the blob diff parameters");
}
if (@difftree > 1) {
- die_error('404 Not Found', "Ambiguous blob diff specification");
+ die_error(400, "Ambiguous blob diff specification");
}
%diffinfo = parse_difftree_raw_line($difftree[0]);
@@ -4826,7 +4838,7 @@ sub git_blobdiff {
'-p', ($format eq 'html' ? "--full-index" : ()),
$hash_parent_base, $hash_base,
"--", (defined $file_parent ? $file_parent : ()), $file_name
- or die_error(undef, "Open git-diff-tree failed");
+ or die_error(500, "Open git-diff-tree failed");
}
# old/legacy style URI
@@ -4862,9 +4874,9 @@ sub git_blobdiff {
open $fd, "-|", git_cmd(), "diff", @diff_opts,
'-p', ($format eq 'html' ? "--full-index" : ()),
$hash_parent, $hash, "--"
- or die_error(undef, "Open git-diff failed");
+ or die_error(500, "Open git-diff failed");
} else {
- die_error('404 Not Found', "Missing one of the blob diff parameters")
+ die_error(400, "Missing one of the blob diff parameters")
unless %diffinfo;
}
@@ -4897,7 +4909,7 @@ sub git_blobdiff {
print "X-Git-Url: " . $cgi->self_url() . "\n\n";
} else {
- die_error(undef, "Unknown blobdiff format");
+ die_error(400, "Unknown blobdiff format");
}
# patch
@@ -4932,10 +4944,8 @@ sub git_blobdiff_plain {
sub git_commitdiff {
my $format = shift || 'html';
$hash ||= $hash_base || "HEAD";
- my %co = parse_commit($hash);
- if (!%co) {
- die_error(undef, "Unknown commit object");
- }
+ my %co = parse_commit($hash)
+ or die_error(404, "Unknown commit object");
# choose format for commitdiff for merge
if (! defined $hash_parent && @{$co{'parents'}} > 1) {
@@ -5017,7 +5027,7 @@ sub git_commitdiff {
open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
"--no-commit-id", "--patch-with-raw", "--full-index",
$hash_parent_param, $hash, "--"
- or die_error(undef, "Open git-diff-tree failed");
+ or die_error(500, "Open git-diff-tree failed");
while (my $line = <$fd>) {
chomp $line;
@@ -5029,10 +5039,10 @@ sub git_commitdiff {
} elsif ($format eq 'plain') {
open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
'-p', $hash_parent_param, $hash, "--"
- or die_error(undef, "Open git-diff-tree failed");
+ or die_error(500, "Open git-diff-tree failed");
} else {
- die_error(undef, "Unknown commitdiff format");
+ die_error(400, "Unknown commitdiff format");
}
# non-textual hash id's can be cached
@@ -5115,19 +5125,15 @@ sub git_history {
$page = 0;
}
my $ftype;
- my %co = parse_commit($hash_base);
- if (!%co) {
- die_error(undef, "Unknown commit object");
- }
+ my %co = parse_commit($hash_base)
+ or die_error(404, "Unknown commit object");
my $refs = git_get_references();
my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
my @commitlist = parse_commits($hash_base, 101, (100 * $page),
- $file_name, "--full-history");
- if (!@commitlist) {
- die_error('404 Not Found', "No such file or directory on given branch");
- }
+ $file_name, "--full-history")
+ or die_error(404, "No such file or directory on given branch");
if (!defined $hash && defined $file_name) {
# some commits could have deleted file in question,
@@ -5141,7 +5147,7 @@ sub git_history {
$ftype = git_get_type($hash);
}
if (!defined $ftype) {
- die_error(undef, "Unknown type of object");
+ die_error(500, "Unknown type of object");
}
my $paging_nav = '';
@@ -5179,19 +5185,16 @@ sub git_history {
}
sub git_search {
- my ($have_search) = gitweb_check_feature('search');
- if (!$have_search) {
- die_error('403 Permission denied', "Permission denied");
- }
+ gitweb_check_feature('search') or die_error(403, "Search is disabled");
if (!defined $searchtext) {
- die_error(undef, "Text field empty");
+ die_error(400, "Text field is empty");
}
if (!defined $hash) {
$hash = git_get_head_hash($project);
}
my %co = parse_commit($hash);
if (!%co) {
- die_error(undef, "Unknown commit object");
+ die_error(404, "Unknown commit object");
}
if (!defined $page) {
$page = 0;
@@ -5201,16 +5204,12 @@ sub git_search {
if ($searchtype eq 'pickaxe') {
# pickaxe may take all resources of your box and run for several minutes
# with every query - so decide by yourself how public you make this feature
- my ($have_pickaxe) = gitweb_check_feature('pickaxe');
- if (!$have_pickaxe) {
- die_error('403 Permission denied', "Permission denied");
- }
+ gitweb_check_feature('pickaxe')
+ or die_error(403, "Pickaxe is disabled");
}
if ($searchtype eq 'grep') {
- my ($have_grep) = gitweb_check_feature('grep');
- if (!$have_grep) {
- die_error('403 Permission denied', "Permission denied");
- }
+ gitweb_check_feature('grep')
+ or die_error(403, "Grep is disabled");
}
git_header_html();
@@ -5484,7 +5483,7 @@ sub git_feed {
# Atom: http://www.atomenabled.org/developers/syndication/
# RSS: http://www.notestips.com/80256B3A007F2692/1/NAMO5P9UPQ
if ($format ne 'rss' && $format ne 'atom') {
- die_error(undef, "Unknown web feed format");
+ die_error(400, "Unknown web feed format");
}
# log/feed of current (HEAD) branch, log of given branch, history of file/directory
--
1.5.6.149.g06c04.dirty
^ permalink raw reply related
* Re: [PATCH] Authentication support for pserver
From: Junio C Hamano @ 2008-06-19 20:14 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Martin Langhoff, Junio C Hamano, git, martyn, martin
In-Reply-To: <861w2tjpev.fsf@cpan.org>
avar@cpan.org (Ævar Arnfjörð Bjarmason) writes:
> ...
> It has been over 3 months since I submitted this patch without anyone
> acting on it. In absence of comment from the Mart[yi]ns could this
> please applied anyway?
Can you send a patch that can be applied (cf. Documentation/SubmittingPatches)?
^ permalink raw reply
* [PATCH v3] gitweb: standarize HTTP status codes
From: Lea Wiemann @ 2008-06-19 20:03 UTC (permalink / raw)
To: git; +Cc: Lea Wiemann
In-Reply-To: <485AAEB9.2080100@gmail.com>
Many error status codes simply default to 403 Forbidden, which is not
correct in most cases. This patch makes gitweb return semantically
correct status codes.
For convenience the die_error function now only takes the status code
without reason as first parameter (e.g. 404 instead of "404 Not
Found"), and it now defaults to 500 (Internal Server Error), even
though the default is not used anywhere.
Also documented status code conventions in die_error.
Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
---
Changes since v2: die_error now adds the reason strings defined by RFC
2616 to the HTTP status code; incorporated Jakub's other suggestions.
Diff to v2 follows.
I didn't use the HTTP_NOT_FOUND etc. suggestion because I found it too
verbose and obtrusive.
Just a friendly reminder, please remember that discussing fairly
trivial changes in-depth might be not a good use of all participants'
time; IOW, sending 340 lines of email about HTTP status codes that
don't *really* matter is probably overkill. (In case you're
interested, the reason why I wrote this patch in the first place is in
order to be able to tell unforeseen errors from expected errors in the
test suite.) I recommend you read http://bikeshed.com/ if you haven't
done so.
Also, increasing the time between patch submission and acceptance
causes added overhead (effort) by itself (i.e. *in addition* to the
time spend on discussing and resending patches); that's because you
cannot base anything on the code without risking merge conflicts.
Please think of that when replying to patches. ;-)
By the way, when you want to make trivial changes, it may be easier
for everyone if you simply implement them and send a follow-up patch
(and it might actually be just as quick as suggesting the changes!).
In your follow-up patch you could provide a diff to the previous
version, and comment on the changes you made (or send it without
comments, even -- oftentimes they're obvious).
Anyways, I hope everyone is happy with this version of the patch.
Diff to v2:
index 3ca45fd..3bc8f69 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2687,9 +2687,11 @@ sub die_error {
my $status = shift || 500;
my $error = shift || "Internal server error";
- # Use a generic "Error" reason, e.g. "404 Error" instead of
- # "404 Not Found". This is permissible by RFC 2616.
- git_header_html("$status Error");
+ my %http_responses = (400 => '400 Bad Request',
+ 403 => '403 Forbidden',
+ 404 => '404 Not Found',
+ 500 => '500 Internal Server Error');
+ git_header_html($http_responses{$status});
print <<EOF;
<div class="page_body">
<br /><br />
@@ -4131,11 +4133,11 @@ sub git_blame {
my $ftype;
gitweb_check_feature('blame')
- or die_error(403, "Blame not allowed");
+ or die_error(403, "Blame view not allowed");
die_error(400, "No file name given") unless $file_name;
$hash_base ||= git_get_head_hash($project);
- die_error(400, "Couldn't find base commit") unless ($hash_base);
+ die_error(404, "Couldn't find base commit") unless ($hash_base);
my %co = parse_commit($hash_base)
or die_error(404, "Commit not found");
if (!defined $hash) {
@@ -4403,7 +4405,7 @@ sub git_tree {
open my $fd, "-|", git_cmd(), "ls-tree", '-z', $hash
or die_error(500, "Open git-ls-tree failed");
my @entries = map { chomp; $_ } <$fd>;
- close $fd or die_error(500, "Reading tree failed");
+ close $fd or die_error(404, "Reading tree failed");
$/ = "\n";
my $refs = git_get_references();
@@ -4641,7 +4643,7 @@ sub git_commit {
$hash, "--"
or die_error(500, "Open git-diff-tree failed");
@difftree = map { chomp; $_ } <$fd>;
- close $fd or die_error(500, "Reading git-diff-tree failed");
+ close $fd or die_error(404, "Reading git-diff-tree failed");
# non-textual hash id's can be cached
my $expires;
@@ -4788,7 +4790,7 @@ sub git_blobdiff {
or die_error(500, "Open git-diff-tree failed");
@difftree = map { chomp; $_ } <$fd>;
close $fd
- or die_error(500, "Reading git-diff-tree failed");
+ or die_error(404, "Reading git-diff-tree failed");
@difftree
or die_error(404, "Blob diff not found");
@@ -4806,7 +4808,7 @@ sub git_blobdiff {
grep { /^:[0-7]{6} [0-7]{6} [0-9a-fA-F]{40} $hash/ }
map { chomp; $_ } <$fd>;
close $fd
- or die_error(500, "Reading git-diff-tree failed");
+ or die_error(404, "Reading git-diff-tree failed");
@difftree
or die_error(404, "Blob diff not found");
gitweb/gitweb.perl | 211 ++++++++++++++++++++++++++--------------------------
1 files changed, 105 insertions(+), 106 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3a7adae..3bc8f69 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -386,7 +386,7 @@ $projects_list ||= $projectroot;
our $action = $cgi->param('a');
if (defined $action) {
if ($action =~ m/[^0-9a-zA-Z\.\-_]/) {
- die_error(undef, "Invalid action parameter");
+ die_error(400, "Invalid action parameter");
}
}
@@ -399,21 +399,21 @@ if (defined $project) {
($export_ok && !(-e "$projectroot/$project/$export_ok")) ||
($strict_export && !project_in_list($project))) {
undef $project;
- die_error(undef, "No such project");
+ die_error(404, "No such project");
}
}
our $file_name = $cgi->param('f');
if (defined $file_name) {
if (!validate_pathname($file_name)) {
- die_error(undef, "Invalid file parameter");
+ die_error(400, "Invalid file parameter");
}
}
our $file_parent = $cgi->param('fp');
if (defined $file_parent) {
if (!validate_pathname($file_parent)) {
- die_error(undef, "Invalid file parent parameter");
+ die_error(400, "Invalid file parent parameter");
}
}
@@ -421,21 +421,21 @@ if (defined $file_parent) {
our $hash = $cgi->param('h');
if (defined $hash) {
if (!validate_refname($hash)) {
- die_error(undef, "Invalid hash parameter");
+ die_error(400, "Invalid hash parameter");
}
}
our $hash_parent = $cgi->param('hp');
if (defined $hash_parent) {
if (!validate_refname($hash_parent)) {
- die_error(undef, "Invalid hash parent parameter");
+ die_error(400, "Invalid hash parent parameter");
}
}
our $hash_base = $cgi->param('hb');
if (defined $hash_base) {
if (!validate_refname($hash_base)) {
- die_error(undef, "Invalid hash base parameter");
+ die_error(400, "Invalid hash base parameter");
}
}
@@ -447,10 +447,10 @@ our @extra_options = $cgi->param('opt');
if (defined @extra_options) {
foreach my $opt (@extra_options) {
if (not exists $allowed_options{$opt}) {
- die_error(undef, "Invalid option parameter");
+ die_error(400, "Invalid option parameter");
}
if (not grep(/^$action$/, @{$allowed_options{$opt}})) {
- die_error(undef, "Invalid option parameter for this action");
+ die_error(400, "Invalid option parameter for this action");
}
}
}
@@ -458,7 +458,7 @@ if (defined @extra_options) {
our $hash_parent_base = $cgi->param('hpb');
if (defined $hash_parent_base) {
if (!validate_refname($hash_parent_base)) {
- die_error(undef, "Invalid hash parent base parameter");
+ die_error(400, "Invalid hash parent base parameter");
}
}
@@ -466,14 +466,14 @@ if (defined $hash_parent_base) {
our $page = $cgi->param('pg');
if (defined $page) {
if ($page =~ m/[^0-9]/) {
- die_error(undef, "Invalid page parameter");
+ die_error(400, "Invalid page parameter");
}
}
our $searchtype = $cgi->param('st');
if (defined $searchtype) {
if ($searchtype =~ m/[^a-z]/) {
- die_error(undef, "Invalid searchtype parameter");
+ die_error(400, "Invalid searchtype parameter");
}
}
@@ -483,7 +483,7 @@ our $searchtext = $cgi->param('s');
our $search_regexp;
if (defined $searchtext) {
if (length($searchtext) < 2) {
- die_error(undef, "At least two characters are required for search parameter");
+ die_error(403, "At least two characters are required for search parameter");
}
$search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext;
}
@@ -580,11 +580,11 @@ if (!defined $action) {
}
}
if (!defined($actions{$action})) {
- die_error(undef, "Unknown action");
+ die_error(400, "Unknown action");
}
if ($action !~ m/^(opml|project_list|project_index)$/ &&
!$project) {
- die_error(undef, "Project needed");
+ die_error(400, "Project needed");
}
$actions{$action}->();
exit;
@@ -1665,7 +1665,7 @@ sub git_get_hash_by_path {
$path =~ s,/+$,,;
open my $fd, "-|", git_cmd(), "ls-tree", $base, "--", $path
- or die_error(undef, "Open git-ls-tree failed");
+ or die_error(500, "Open git-ls-tree failed");
my $line = <$fd>;
close $fd or return undef;
@@ -2127,7 +2127,7 @@ sub parse_commit {
"--max-count=1",
$commit_id,
"--",
- or die_error(undef, "Open git-rev-list failed");
+ or die_error(500, "Open git-rev-list failed");
%co = parse_commit_text(<$fd>, 1);
close $fd;
@@ -2152,7 +2152,7 @@ sub parse_commits {
$commit_id,
"--",
($filename ? ($filename) : ())
- or die_error(undef, "Open git-rev-list failed");
+ or die_error(500, "Open git-rev-list failed");
while (my $line = <$fd>) {
my %co = parse_commit_text($line);
push @cos, \%co;
@@ -2672,11 +2672,26 @@ sub git_footer_html {
"</html>";
}
+# die_error(<http_status_code>, <error_message>)
+# Example: die_error(404, 'Hash not found')
+# By convention, use the following status codes (as defined in RFC 2616):
+# 400: Invalid or missing CGI parameters, or
+# requested object exists but has wrong type.
+# 403: Requested feature (like "pickaxe" or "snapshot") not enabled on
+# this server or project.
+# 404: Requested object/revision/project doesn't exist.
+# 500: The server isn't configured properly, or
+# an internal error occurred (e.g. failed assertions caused by bugs), or
+# an unknown error occurred (e.g. the git binary died unexpectedly).
sub die_error {
- my $status = shift || "403 Forbidden";
- my $error = shift || "Malformed query, file missing or permission denied";
-
- git_header_html($status);
+ my $status = shift || 500;
+ my $error = shift || "Internal server error";
+
+ my %http_responses = (400 => '400 Bad Request',
+ 403 => '403 Forbidden',
+ 404 => '404 Not Found',
+ 500 => '500 Internal Server Error');
+ git_header_html($http_responses{$status});
print <<EOF;
<div class="page_body">
<br /><br />
@@ -3924,12 +3939,12 @@ sub git_search_grep_body {
sub git_project_list {
my $order = $cgi->param('o');
if (defined $order && $order !~ m/none|project|descr|owner|age/) {
- die_error(undef, "Unknown order parameter");
+ die_error(400, "Unknown order parameter");
}
my @list = git_get_projects_list();
if (!@list) {
- die_error(undef, "No projects found");
+ die_error(404, "No projects found");
}
git_header_html();
@@ -3947,12 +3962,12 @@ sub git_project_list {
sub git_forks {
my $order = $cgi->param('o');
if (defined $order && $order !~ m/none|project|descr|owner|age/) {
- die_error(undef, "Unknown order parameter");
+ die_error(400, "Unknown order parameter");
}
my @list = git_get_projects_list($project);
if (!@list) {
- die_error(undef, "No forks found");
+ die_error(404, "No forks found");
}
git_header_html();
@@ -4081,7 +4096,7 @@ sub git_tag {
my %tag = parse_tag($hash);
if (! %tag) {
- die_error(undef, "Unknown tag object");
+ die_error(404, "Unknown tag object");
}
git_print_header_div('commit', esc_html($tag{'name'}), $hash);
@@ -4117,26 +4132,25 @@ sub git_blame {
my $fd;
my $ftype;
- my ($have_blame) = gitweb_check_feature('blame');
- if (!$have_blame) {
- die_error('403 Permission denied', "Permission denied");
- }
- die_error('404 Not Found', "File name not defined") if (!$file_name);
+ gitweb_check_feature('blame')
+ or die_error(403, "Blame view not allowed");
+
+ die_error(400, "No file name given") unless $file_name;
$hash_base ||= git_get_head_hash($project);
- die_error(undef, "Couldn't find base commit") unless ($hash_base);
+ die_error(404, "Couldn't find base commit") unless ($hash_base);
my %co = parse_commit($hash_base)
- or die_error(undef, "Reading commit failed");
+ or die_error(404, "Commit not found");
if (!defined $hash) {
$hash = git_get_hash_by_path($hash_base, $file_name, "blob")
- or die_error(undef, "Error looking up file");
+ or die_error(404, "Error looking up file");
}
$ftype = git_get_type($hash);
if ($ftype !~ "blob") {
- die_error('400 Bad Request', "Object is not a blob");
+ die_error(400, "Object is not a blob");
}
open ($fd, "-|", git_cmd(), "blame", '-p', '--',
$file_name, $hash_base)
- or die_error(undef, "Open git-blame failed");
+ or die_error(500, "Open git-blame failed");
git_header_html();
my $formats_nav =
$cgi->a({-href => href(action=>"blob", -replay=>1)},
@@ -4198,7 +4212,7 @@ HTML
print "</td>\n";
}
open (my $dd, "-|", git_cmd(), "rev-parse", "$full_rev^")
- or die_error(undef, "Open git-rev-parse failed");
+ or die_error(500, "Open git-rev-parse failed");
my $parent_commit = <$dd>;
close $dd;
chomp($parent_commit);
@@ -4255,9 +4269,9 @@ sub git_blob_plain {
if (defined $file_name) {
my $base = $hash_base || git_get_head_hash($project);
$hash = git_get_hash_by_path($base, $file_name, "blob")
- or die_error(undef, "Error lookup file");
+ or die_error(404, "Cannot find file");
} else {
- die_error(undef, "No file name defined");
+ die_error(400, "No file name defined");
}
} elsif ($hash =~ m/^[0-9a-fA-F]{40}$/) {
# blobs defined by non-textual hash id's can be cached
@@ -4265,7 +4279,7 @@ sub git_blob_plain {
}
open my $fd, "-|", git_cmd(), "cat-file", "blob", $hash
- or die_error(undef, "Open git-cat-file blob '$hash' failed");
+ or die_error(500, "Open git-cat-file blob '$hash' failed");
# content-type (can include charset)
$type = blob_contenttype($fd, $file_name, $type);
@@ -4297,9 +4311,9 @@ sub git_blob {
if (defined $file_name) {
my $base = $hash_base || git_get_head_hash($project);
$hash = git_get_hash_by_path($base, $file_name, "blob")
- or die_error(undef, "Error lookup file");
+ or die_error(404, "Cannot find file");
} else {
- die_error(undef, "No file name defined");
+ die_error(400, "No file name defined");
}
} elsif ($hash =~ m/^[0-9a-fA-F]{40}$/) {
# blobs defined by non-textual hash id's can be cached
@@ -4308,7 +4322,7 @@ sub git_blob {
my ($have_blame) = gitweb_check_feature('blame');
open my $fd, "-|", git_cmd(), "cat-file", "blob", $hash
- or die_error(undef, "Couldn't cat $file_name, $hash");
+ or die_error(500, "Couldn't cat $file_name, $hash");
my $mimetype = blob_mimetype($fd, $file_name);
if ($mimetype !~ m!^(?:text/|image/(?:gif|png|jpeg)$)! && -B $fd) {
close $fd;
@@ -4389,9 +4403,9 @@ sub git_tree {
}
$/ = "\0";
open my $fd, "-|", git_cmd(), "ls-tree", '-z', $hash
- or die_error(undef, "Open git-ls-tree failed");
+ or die_error(500, "Open git-ls-tree failed");
my @entries = map { chomp; $_ } <$fd>;
- close $fd or die_error(undef, "Reading tree failed");
+ close $fd or die_error(404, "Reading tree failed");
$/ = "\n";
my $refs = git_get_references();
@@ -4481,16 +4495,16 @@ sub git_snapshot {
my $format = $cgi->param('sf');
if (!@supported_fmts) {
- die_error('403 Permission denied', "Permission denied");
+ die_error(403, "Snapshots not allowed");
}
# default to first supported snapshot format
$format ||= $supported_fmts[0];
if ($format !~ m/^[a-z0-9]+$/) {
- die_error(undef, "Invalid snapshot format parameter");
+ die_error(400, "Invalid snapshot format parameter");
} elsif (!exists($known_snapshot_formats{$format})) {
- die_error(undef, "Unknown snapshot format");
+ die_error(400, "Unknown snapshot format");
} elsif (!grep($_ eq $format, @supported_fmts)) {
- die_error(undef, "Unsupported snapshot format");
+ die_error(403, "Unsupported snapshot format");
}
if (!defined $hash) {
@@ -4518,7 +4532,7 @@ sub git_snapshot {
-status => '200 OK');
open my $fd, "-|", $cmd
- or die_error(undef, "Execute git-archive failed");
+ or die_error(500, "Execute git-archive failed");
binmode STDOUT, ':raw';
print <$fd>;
binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi
@@ -4586,10 +4600,8 @@ sub git_log {
sub git_commit {
$hash ||= $hash_base || "HEAD";
- my %co = parse_commit($hash);
- if (!%co) {
- die_error(undef, "Unknown commit object");
- }
+ my %co = parse_commit($hash)
+ or die_error(404, "Unknown commit object");
my %ad = parse_date($co{'author_epoch'}, $co{'author_tz'});
my %cd = parse_date($co{'committer_epoch'}, $co{'committer_tz'});
@@ -4629,9 +4641,9 @@ sub git_commit {
@diff_opts,
(@$parents <= 1 ? $parent : '-c'),
$hash, "--"
- or die_error(undef, "Open git-diff-tree failed");
+ or die_error(500, "Open git-diff-tree failed");
@difftree = map { chomp; $_ } <$fd>;
- close $fd or die_error(undef, "Reading git-diff-tree failed");
+ close $fd or die_error(404, "Reading git-diff-tree failed");
# non-textual hash id's can be cached
my $expires;
@@ -4724,33 +4736,33 @@ sub git_object {
open my $fd, "-|", quote_command(
git_cmd(), 'cat-file', '-t', $object_id) . ' 2> /dev/null'
- or die_error('404 Not Found', "Object does not exist");
+ or die_error(404, "Object does not exist");
$type = <$fd>;
chomp $type;
close $fd
- or die_error('404 Not Found', "Object does not exist");
+ or die_error(404, "Object does not exist");
# - hash_base and file_name
} elsif ($hash_base && defined $file_name) {
$file_name =~ s,/+$,,;
system(git_cmd(), "cat-file", '-e', $hash_base) == 0
- or die_error('404 Not Found', "Base object does not exist");
+ or die_error(404, "Base object does not exist");
# here errors should not hapen
open my $fd, "-|", git_cmd(), "ls-tree", $hash_base, "--", $file_name
- or die_error(undef, "Open git-ls-tree failed");
+ or die_error(500, "Open git-ls-tree failed");
my $line = <$fd>;
close $fd;
#'100644 blob 0fa3f3a66fb6a137f6ec2c19351ed4d807070ffa panic.c'
unless ($line && $line =~ m/^([0-9]+) (.+) ([0-9a-fA-F]{40})\t/) {
- die_error('404 Not Found', "File or directory for given base does not exist");
+ die_error(404, "File or directory for given base does not exist");
}
$type = $2;
$hash = $3;
} else {
- die_error('404 Not Found', "Not enough information to find object");
+ die_error(400, "Not enough information to find object");
}
print $cgi->redirect(-uri => href(action=>$type, -full=>1,
@@ -4775,12 +4787,12 @@ sub git_blobdiff {
open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
$hash_parent_base, $hash_base,
"--", (defined $file_parent ? $file_parent : ()), $file_name
- or die_error(undef, "Open git-diff-tree failed");
+ or die_error(500, "Open git-diff-tree failed");
@difftree = map { chomp; $_ } <$fd>;
close $fd
- or die_error(undef, "Reading git-diff-tree failed");
+ or die_error(404, "Reading git-diff-tree failed");
@difftree
- or die_error('404 Not Found', "Blob diff not found");
+ or die_error(404, "Blob diff not found");
} elsif (defined $hash &&
$hash =~ /[0-9a-fA-F]{40}/) {
@@ -4789,23 +4801,23 @@ sub git_blobdiff {
# read filtered raw output
open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
$hash_parent_base, $hash_base, "--"
- or die_error(undef, "Open git-diff-tree failed");
+ or die_error(500, "Open git-diff-tree failed");
@difftree =
# ':100644 100644 03b21826... 3b93d5e7... M ls-files.c'
# $hash == to_id
grep { /^:[0-7]{6} [0-7]{6} [0-9a-fA-F]{40} $hash/ }
map { chomp; $_ } <$fd>;
close $fd
- or die_error(undef, "Reading git-diff-tree failed");
+ or die_error(404, "Reading git-diff-tree failed");
@difftree
- or die_error('404 Not Found', "Blob diff not found");
+ or die_error(404, "Blob diff not found");
} else {
- die_error('404 Not Found', "Missing one of the blob diff parameters");
+ die_error(400, "Missing one of the blob diff parameters");
}
if (@difftree > 1) {
- die_error('404 Not Found', "Ambiguous blob diff specification");
+ die_error(400, "Ambiguous blob diff specification");
}
%diffinfo = parse_difftree_raw_line($difftree[0]);
@@ -4826,7 +4838,7 @@ sub git_blobdiff {
'-p', ($format eq 'html' ? "--full-index" : ()),
$hash_parent_base, $hash_base,
"--", (defined $file_parent ? $file_parent : ()), $file_name
- or die_error(undef, "Open git-diff-tree failed");
+ or die_error(500, "Open git-diff-tree failed");
}
# old/legacy style URI
@@ -4862,9 +4874,9 @@ sub git_blobdiff {
open $fd, "-|", git_cmd(), "diff", @diff_opts,
'-p', ($format eq 'html' ? "--full-index" : ()),
$hash_parent, $hash, "--"
- or die_error(undef, "Open git-diff failed");
+ or die_error(500, "Open git-diff failed");
} else {
- die_error('404 Not Found', "Missing one of the blob diff parameters")
+ die_error(400, "Missing one of the blob diff parameters")
unless %diffinfo;
}
@@ -4897,7 +4909,7 @@ sub git_blobdiff {
print "X-Git-Url: " . $cgi->self_url() . "\n\n";
} else {
- die_error(undef, "Unknown blobdiff format");
+ die_error(400, "Unknown blobdiff format");
}
# patch
@@ -4932,10 +4944,8 @@ sub git_blobdiff_plain {
sub git_commitdiff {
my $format = shift || 'html';
$hash ||= $hash_base || "HEAD";
- my %co = parse_commit($hash);
- if (!%co) {
- die_error(undef, "Unknown commit object");
- }
+ my %co = parse_commit($hash)
+ or die_error(404, "Unknown commit object");
# choose format for commitdiff for merge
if (! defined $hash_parent && @{$co{'parents'}} > 1) {
@@ -5017,7 +5027,7 @@ sub git_commitdiff {
open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
"--no-commit-id", "--patch-with-raw", "--full-index",
$hash_parent_param, $hash, "--"
- or die_error(undef, "Open git-diff-tree failed");
+ or die_error(500, "Open git-diff-tree failed");
while (my $line = <$fd>) {
chomp $line;
@@ -5029,10 +5039,10 @@ sub git_commitdiff {
} elsif ($format eq 'plain') {
open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
'-p', $hash_parent_param, $hash, "--"
- or die_error(undef, "Open git-diff-tree failed");
+ or die_error(500, "Open git-diff-tree failed");
} else {
- die_error(undef, "Unknown commitdiff format");
+ die_error(400, "Unknown commitdiff format");
}
# non-textual hash id's can be cached
@@ -5115,19 +5125,15 @@ sub git_history {
$page = 0;
}
my $ftype;
- my %co = parse_commit($hash_base);
- if (!%co) {
- die_error(undef, "Unknown commit object");
- }
+ my %co = parse_commit($hash_base)
+ or die_error(404, "Unknown commit object");
my $refs = git_get_references();
my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
my @commitlist = parse_commits($hash_base, 101, (100 * $page),
- $file_name, "--full-history");
- if (!@commitlist) {
- die_error('404 Not Found', "No such file or directory on given branch");
- }
+ $file_name, "--full-history")
+ or die_error(404, "No such file or directory on given branch");
if (!defined $hash && defined $file_name) {
# some commits could have deleted file in question,
@@ -5141,7 +5147,7 @@ sub git_history {
$ftype = git_get_type($hash);
}
if (!defined $ftype) {
- die_error(undef, "Unknown type of object");
+ die_error(500, "Unknown type of object");
}
my $paging_nav = '';
@@ -5179,19 +5185,16 @@ sub git_history {
}
sub git_search {
- my ($have_search) = gitweb_check_feature('search');
- if (!$have_search) {
- die_error('403 Permission denied', "Permission denied");
- }
+ gitweb_check_feature('search') or die_error(403, "Search is disabled");
if (!defined $searchtext) {
- die_error(undef, "Text field empty");
+ die_error(400, "Text field is empty");
}
if (!defined $hash) {
$hash = git_get_head_hash($project);
}
my %co = parse_commit($hash);
if (!%co) {
- die_error(undef, "Unknown commit object");
+ die_error(404, "Unknown commit object");
}
if (!defined $page) {
$page = 0;
@@ -5201,16 +5204,12 @@ sub git_search {
if ($searchtype eq 'pickaxe') {
# pickaxe may take all resources of your box and run for several minutes
# with every query - so decide by yourself how public you make this feature
- my ($have_pickaxe) = gitweb_check_feature('pickaxe');
- if (!$have_pickaxe) {
- die_error('403 Permission denied', "Permission denied");
- }
+ gitweb_check_feature('pickaxe')
+ or die_error(403, "Pickaxe is disabled");
}
if ($searchtype eq 'grep') {
- my ($have_grep) = gitweb_check_feature('grep');
- if (!$have_grep) {
- die_error('403 Permission denied', "Permission denied");
- }
+ gitweb_check_feature('grep')
+ or die_error(403, "Grep is disabled");
}
git_header_html();
@@ -5484,7 +5483,7 @@ sub git_feed {
# Atom: http://www.atomenabled.org/developers/syndication/
# RSS: http://www.notestips.com/80256B3A007F2692/1/NAMO5P9UPQ
if ($format ne 'rss' && $format ne 'atom') {
- die_error(undef, "Unknown web feed format");
+ die_error(400, "Unknown web feed format");
}
# log/feed of current (HEAD) branch, log of given branch, history of file/directory
--
1.5.6.149.g06c04.dirty
^ permalink raw reply related
* Re: [PATCH v2] Make git_dir a path relative to work_tree in setup_work_tree()
From: Linus Torvalds @ 2008-06-19 19:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Daniel Barkalow, Linus Torvalds, git
In-Reply-To: <7vod5xuvtj.fsf@gitster.siamese.dyndns.org>
On Thu, 19 Jun 2008, Junio C Hamano wrote:
>
> Other than that, I think the change is Ok, but as a "performance tweak",
> it should be backed by some numbers, please?
Daniel's patch didn't apply for me as-is, so I recreated it with some
differences (appended), and here are the numbers from ten runs each.
There is some IO for me - probably due to more-or-less random flushing of
the journal - so the variation is bigger than I'd like, but whatever:
Before:
real 0m8.135s
real 0m7.933s
real 0m8.080s
real 0m7.954s
real 0m7.949s
real 0m8.112s
real 0m7.934s
real 0m8.059s
real 0m7.979s
real 0m8.038s
After:
real 0m7.685s
real 0m7.968s
real 0m7.703s
real 0m7.850s
real 0m7.995s
real 0m7.817s
real 0m7.963s
real 0m7.955s
real 0m7.848s
real 0m7.969s
Now, going by "best of ten" (on the assumption that the longer numbers
are all due to IO), I'm saying a 7.933s -> 7.685s reduction, and it does
seem to be outside of the noise (ie the "after" case never broke 8s, while
the "before" case did so half the time).
So looks like about 3% to me.
Doing it for a slightly smaller test-case (just the "arch" subdirectory)
gets more stable numbers probably due to not filling the journal with
metadata updates, so we have:
Before:
real 0m1.633s
real 0m1.633s
real 0m1.633s
real 0m1.632s
real 0m1.632s
real 0m1.630s
real 0m1.634s
real 0m1.631s
real 0m1.632s
real 0m1.632s
After:
real 0m1.610s
real 0m1.609s
real 0m1.610s
real 0m1.608s
real 0m1.607s
real 0m1.610s
real 0m1.609s
real 0m1.611s
real 0m1.608s
real 0m1.611s
where I'ld just take the averages and say 1.632 vs 1.610, which is just
over 1% peformance improvement.
So it's not in the noise, but it's not as big as I initially thought and
measured.
(That said, it obviously depends on how deep the working directory path is
too, and whether it is behind NFS or something else that might need to
cause more work to look up).
Modified/updated patch appended.
Linus
----
cache.h | 1 +
path.c | 17 +++++++++++++++++
setup.c | 3 ++-
3 files changed, 20 insertions(+), 1 deletions(-)
diff --git a/cache.h b/cache.h
index 01c8502..29aae0c 100644
--- a/cache.h
+++ b/cache.h
@@ -526,6 +526,7 @@ static inline int is_absolute_path(const char *path)
}
const char *make_absolute_path(const char *path);
const char *make_nonrelative_path(const char *path);
+const char *make_relative_path(const char *abs, const char *base);
/* Read and unpack a sha1 file into memory, write memory to a sha1 file */
extern int sha1_object_info(const unsigned char *, unsigned long *);
diff --git a/path.c b/path.c
index 7a35a26..6e3df18 100644
--- a/path.c
+++ b/path.c
@@ -330,6 +330,23 @@ const char *make_nonrelative_path(const char *path)
/* We allow "recursive" symbolic links. Only within reason, though. */
#define MAXDEPTH 5
+const char *make_relative_path(const char *abs, const char *base)
+{
+ static char buf[PATH_MAX + 1];
+ int baselen;
+ if (!base)
+ return abs;
+ baselen = strlen(base);
+ if (prefixcmp(abs, base))
+ return abs;
+ if (abs[baselen] == '/')
+ baselen++;
+ else if (base[baselen - 1] != '/')
+ return abs;
+ strcpy(buf, abs + baselen);
+ return buf;
+}
+
const char *make_absolute_path(const char *path)
{
static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];
diff --git a/setup.c b/setup.c
index d630e37..3b111ea 100644
--- a/setup.c
+++ b/setup.c
@@ -292,9 +292,10 @@ void setup_work_tree(void)
work_tree = get_git_work_tree();
git_dir = get_git_dir();
if (!is_absolute_path(git_dir))
- set_git_dir(make_absolute_path(git_dir));
+ git_dir = make_absolute_path(git_dir);
if (!work_tree || chdir(work_tree))
die("This operation must be run in a work tree");
+ set_git_dir(make_relative_path(git_dir, work_tree));
initialized = 1;
}
^ permalink raw reply related
* Re: [PATCH] Documentation: Simplify git-rev-parse's example
From: Junio C Hamano @ 2008-06-19 19:33 UTC (permalink / raw)
To: Jon Loeliger; +Cc: Pieter de Bie, Git Mailinglist
In-Reply-To: <485AAF82.3030209@freescale.com>
Jon Loeliger <jdl@freescale.com> writes:
> Pieter de Bie wrote:
>> This example was overly complex and therefore confusing.
>> The commits have been renamed to start the oldest commit with "A"
>> and working up from there. Also, this removes some commits so the graph
>> is simpler. Finally the graph has been reversed in direction to make it
>> more like gitk.
>>
>> Signed-off-by: Pieter de Bie <pdebie@ai.rug.nl>
>> ---
>>
>> This was created after some discussion in #git about how this was confusing.
>> The consesus was that this example is better.
>
> How is this a vast improvement?
>
> I could see that inverting it top-to-bottom would
> be more consistent with gitk or show-branch output.
> Your example doesn't have a 3-parent commit, though,
> and it isn't _that_ much simpler otherwise...
>
> So this is really better _how_?
>
> Oh, right, of course. It removes my name. Got it. :-)
I agree that the patch should have just flipped the tree upside down
without changing the shape of the history the section talks about.
Yet another improvement would have been turning it sideways, not upside
down, because that is how we typically write history in our documentation
(time flows from left to right -- see e.g. git-rebase.txt).
I happen to think the last point you raise is an improvement. It will
quickly become unreadble after a while if we credit individual authors for
every paragraph in-text, and it always bothered me to see somebody's name
(don't get me wrong -- this is not because it is your name nor because it
is not my name, but because it _is_ a name), there but I wasn't bold
enough to remove it without discussion.
^ permalink raw reply
* Re: [PATCH] Authentication support for pserver
From: Junio C Hamano @ 2008-06-19 19:21 UTC (permalink / raw)
To: Martin Langhoff
Cc: Ævar Arnfjörð Bjarmason, Junio C Hamano, git,
martyn, martin
In-Reply-To: <46a038f90806191200n237633u80f0be736e8d227b@mail.gmail.com>
"Martin Langhoff" <martin.langhoff@gmail.com> writes:
> On Thu, Jun 19, 2008 at 1:38 PM, Ævar Arnfjörð Bjarmason <avar@cpan.org> wrote:
>>> 1. http://git.nix.is/?p=avar/git;a=commitdiff;h=60f893bd9fe329bd5cf8ec513d10ec00e85feb2c
>>
>> It has been over 3 months since I submitted this patch without anyone
>> acting on it. In absence of comment from the Mart[yi]ns could this
>> please applied anyway?
>
> Sorry. My opinion on this is a mild ACK on the technical front, and a
> bit of a frown ("do we really want to offer this?"). Specifically, my
> concern goes beyond what you can do against a pserver service - git
> hooks can be targeted with this. "Warning: as safe as telnet" ;-)
"As safe as telnet" is Ok as long as it is strictly optional. What I
would primarily be worried about is if this change is a no-op for existing
users, when they chose not to enable the new feature.
^ permalink raw reply
* Re: [PATCH] graph.c: make many functions static
From: Junio C Hamano @ 2008-06-19 19:16 UTC (permalink / raw)
To: しらいしななこ
Cc: git, Adam Simpkins
In-Reply-To: <20080619082110.6117@nanako3.lavabit.com>
しらいしななこ <nanako3@lavabit.com> writes:
> These function are not used anywhere. Also removes graph_release()
> that is never called.
>
> Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>
I CCed Adam, who is the primary author in this area.
> ---
> graph.c | 57 +++++++++++++++++++++++++++++++++++++++++++--------------
> graph.h | 40 ----------------------------------------
> 2 files changed, 43 insertions(+), 54 deletions(-)
>
> diff --git a/graph.c b/graph.c
> index e2633f8..5f82170 100644
> --- a/graph.c
> +++ b/graph.c
> @@ -4,6 +4,43 @@
> #include "diff.h"
> #include "revision.h"
>
> +/* Internal API */
> + ...
> +static int graph_next_line(struct git_graph *graph, struct strbuf *sb);
> +static void graph_padding_line(struct git_graph *graph, struct strbuf *sb);
> +static void graph_show_strbuf(struct git_graph *graph, struct strbuf const *sb);
I think these are probably fine, not in the sense that nobody calls these
functions _right now_ but in the sense that I do not foresee a calling
sequence outside the graph.c internal that needs to call these directly,
instead of calling graph_show_*() functions that use these.
> @@ -180,14 +217,6 @@ struct git_graph *graph_init(struct rev_info *opt)
> return graph;
> }
>
> -void graph_release(struct git_graph *graph)
> -{
> - free(graph->columns);
> - free(graph->new_columns);
> - free(graph->mapping);
> - free(graph);
> -}
But I do not think this is right. The current lack of caller of this
clean-up function simply means the current users are leaking. I think
they are all of "set up rev_info, do a lengthy operation and exit" pattern
and clean-up immediately before exit is often omitted as unnecessary, but
if we had a clean-up function for the revision API that function would
call this one. I'd rather leave this in place, and let libification
minded people figure out the cleanest places and ways to make this
called.
Other three clean-ups looked Ok to me. Thanks.
^ permalink raw reply
* Re: [PATCH] Add option to git-branch to set up automatic rebasing
From: Junio C Hamano @ 2008-06-19 19:14 UTC (permalink / raw)
To: Björn Steinbrink; +Cc: Johannes Schindelin, Pieter de Bie, Git Mailinglist
In-Reply-To: <20080619154350.GA21625@atjola.homenet>
Björn Steinbrink <B.Steinbrink@gmx.de> writes:
> On 2008.06.19 15:00:19 +0100, Johannes Schindelin wrote:
>> Hi,
>>
>> On Thu, 19 Jun 2008, Pieter de Bie wrote:
>>
>> > This functionality was actually introduced in
>> > 0a02186f924aee1bd69f18ed01f645aa332ce0d1, but can only be activated by the
>> > configuration flag. Now we can also setup auto rebasing using the --rebase
>> > flag in git-branch or git-checkout, similar to how --track works.
>>
>> How about "--rebasing"? I would scratch my head a bit how a new branch
>> and a rebase would go together.
>
> Hm, --rebasing sounds weird to me as well. Maybe --track=merge and
> --track=rebase, with --track being equal to --track=merge?
That looks like the best wording so far, although I suspect that the true
reason why all of the above sounds confusing may be because the concept
itself is not clear.
^ permalink raw reply
* Re: [PATCH] Documentation: Simplify git-rev-parse's example
From: Jon Loeliger @ 2008-06-19 19:12 UTC (permalink / raw)
To: Pieter de Bie; +Cc: Git Mailinglist, Junio C Hamano
In-Reply-To: <1213873976-4192-1-git-send-email-pdebie@ai.rug.nl>
Pieter de Bie wrote:
> This example was overly complex and therefore confusing.
> The commits have been renamed to start the oldest commit with "A"
> and working up from there. Also, this removes some commits so the graph
> is simpler. Finally the graph has been reversed in direction to make it
> more like gitk.
>
> Signed-off-by: Pieter de Bie <pdebie@ai.rug.nl>
> ---
>
> This was created after some discussion in #git about how this was confusing.
> The consesus was that this example is better.
How is this a vast improvement?
I could see that inverting it top-to-bottom would
be more consistent with gitk or show-branch output.
Your example doesn't have a 3-parent commit, though,
and it isn't _that_ much simpler otherwise...
So this is really better _how_?
Oh, right, of course. It removes my name. Got it. :-)
jdl
^ permalink raw reply
* Re: [PATCH v2] gitweb: standarize HTTP status codes
From: Lea Wiemann @ 2008-06-19 19:08 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <m3y752melj.fsf_-_@localhost.localdomain>
Jakub Narebski wrote:
> Lea Wiemann <lewiemann@gmail.com> writes:
>> For convenience the die_error function now only takes the status code
>> without reason as first parameter (e.g. 404 instead of "404 Not Found")
>
> _Whose_ convenience?
The developer's convenience of course. It's plain redundant.
> * I don't think that RFC 2616 allows blanket replacing reason phrase
> by generic "Error",
> * Test::WWW::Mechanize displays both HTTP error status code and
> reason phrase when get_ok(...) fails:
> * From the point of view of someone examinimg gitweb.perl code, 400,
> 403, 404, 500 are _magic numbers_;
I think we're really arguing about the color of the bikeshed here. IMO
we're not stretching RFC 2616 too much by putting "Error" there (since
reason codes don't matter on a technical level), and the status codes
make enough sense to me (and I'm not even a web developer) that I'm not
concerned about readability.
I don't think your constants a la HTTP_INVALID are a good idea (I
remember the status codes in a year, but maybe not the constants);
die_error could figure out the right reason code using a hash. Either
way will be fine with me though.
Look, I really don't care about this and I think it's plain irrelevant
-- do you mind fixing the patch with whatever solution you prefer and
resend it? I'd rather spend my time on getting caching implemented, but
if you insist on me changing this myself, please let me know.
>> Also documented status code conventions in die_error.
>
> Putting copy of those status code conventions in the commit message,
> and not only in the comment section, would be also good idea.
*shrugs* I'd rather not duplicate, and it's in the code anyway.
>> - die_error(undef, "No such project");
>> + die_error(404, "No such project");
>
> Here is one thing worth considering: if project exists, but is
> excluded due to either $export_ok or $strict_export being set,
> should we use '404 Not Found' or '403 Forbidden'?
No, because (a) we'd have add code to check this (so it should be in a
separate patch), and (b) I don't think we even want to return 403, since
(as you say) it would reveal the existence of a project on the server.
Project names can contain sensitive/confidential information though.
>> - die_error(undef, "At least two characters are required for search parameter");
>> + die_error(403, "At least two characters are required for search parameter");
>
> Should gitweb use there '403 Forbidden', or '400 Bad Request'?
> This is failing static validation of CGI parameters, not a matter of
> some permissions...
I used 403 in the sense of "sorry, we don't have shorter search strings
activated for performance reasons". The '2' could even become
configurable. 400 is fine too, though, I don't care.
> *All* "open ... or die_error" constructs should use '500 Internal
> Server Error', as the only errors that can be detected on open are
> very serious, server related ones:
Right, thanks for pointing this out.
>> - my ($have_blame) = gitweb_check_feature('blame');
>> - if (!$have_blame) {
>> - die_error('403 Permission denied', "Permission denied");
>> - }
>> + gitweb_check_feature('blame')
>> + or die_error(403, "Blame not allowed");
>
> That is a bit separate change, i.e. better explanation of an error
> (although I'd say rather "Blame view not allowed").
>
> But what about security reasons?
Separate change: I don't think this has to be a separate patch. ;-)
"Blame view not allowed": Fine with me.
Security concerns: I don't see any, and anyway you can probably infer
from getting 403 on a=blame that it's disabled.
>> - die_error(undef, "Couldn't find base commit") unless ($hash_base);
>> + die_error(400, "Couldn't find base commit") unless ($hash_base);
>
> Wound't it be '404 Not Found', as the explanation suggests?
Yup, right.
>> - close $fd or die_error(undef, "Reading tree failed");
>> + close $fd or die_error(500, "Reading tree failed");
>
> Not O.K. Barring errors in gitweb code this might happen when
> [X Y Z]. All those are clearly 4xx _client_ errors,
I haven't verified that, so until we have better error handling I prefer
500, but I really won't bother objecting to 404. (Same goes for all
other occurrences of 500 you quoted, which I've snipped.) FWIW I'm
assuming that once gitweb uses the new API, that error handling code
will go away anyway.
> One should examine URL for mistakes, not contact server admin here...
I don't think that'll be a practical concern. ;-)
>> - die_error('404 Not Found', "Not enough information to find object");
>> + die_error(400, "Not enough information to find object");
>
> I'm not sure if it should be '400 Bad Request' or '404 Not Found' here.
It's about missing CGI parameters, so 400 should be fine.
>> @difftree
>> - or die_error('404 Not Found', "Blob diff not found");
>> + or die_error(404, "Blob diff not found");
>
> This, if I am not mistaken, can happen only if path limiters doesn'
> catch anything.
Your sentence doesn't quite parse -- why is 404 wrong?
>> - die_error('404 Not Found', "Ambiguous blob diff specification");
>> + die_error(400, "Ambiguous blob diff specification");
>
> Or perhaps '404 Not Dound' here?
Either way is fine.
>> - die_error('404 Not Found', "Missing one of the blob diff parameters")
>> + die_error(400, "Missing one of the blob diff parameters")
>> unless %diffinfo;
>
> The "unless %diffinfo" makes it look more like '404 Not Found'...
Looking at the preceding code (the if statement) I believe diffinfo
being false doesn't indicate 404 but actually a missing CGI parameter
(as the error message states).
>> if (!defined $ftype) {
>> - die_error(undef, "Unknown type of object");
>> + die_error(500, "Unknown type of object");
>
> Errr... shouldn't be '400 Bad Request' here, per convention?
Nope, we didn't get *anything* back, so something weird happened. 500.
-- Lea
^ permalink raw reply
* Re: [PATCH] Authentication support for pserver
From: Martin Langhoff @ 2008-06-19 19:00 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Junio C Hamano, git, martyn, martin
In-Reply-To: <861w2tjpev.fsf@cpan.org>
On Thu, Jun 19, 2008 at 1:38 PM, Ævar Arnfjörð Bjarmason <avar@cpan.org> wrote:
>> 1. http://git.nix.is/?p=avar/git;a=commitdiff;h=60f893bd9fe329bd5cf8ec513d10ec00e85feb2c
>
> It has been over 3 months since I submitted this patch without anyone
> acting on it. In absence of comment from the Mart[yi]ns could this
> please applied anyway?
Sorry. My opinion on this is a mild ACK on the technical front, and a
bit of a frown ("do we really want to offer this?"). Specifically, my
concern goes beyond what you can do against a pserver service - git
hooks can be targeted with this. "Warning: as safe as telnet" ;-)
cheers,
m
--
martin.langhoff@gmail.com
martin@laptop.org -- School Server Architect
- ask interesting questions
- don't get distracted with shiny stuff - working code first
- http://wiki.laptop.org/go/User:Martinlanghoff
^ permalink raw reply
* Re: git pull error message woes
From: Daniel Barkalow @ 2008-06-19 18:57 UTC (permalink / raw)
To: Matthias Kestenholz; +Cc: Git Mailing List
In-Reply-To: <1213860773.6444.9.camel@localhost>
On Thu, 19 Jun 2008, Matthias Kestenholz wrote:
> Hi,
>
> I noticed strange behavior while pulling git.git today (this isn't new,
> it just occurred to me for the first time today that there is something
> wrong going on)
>
> I run the 'pu' branch most of the time, and do not create a local branch
> because 'pu' is constantly rebased. I just run git checkout origin/pu
> after pulling (I know I should fetch if I don't want to fetch+merge, but
> it's hard to retrain the fingers)
>
> Although I am on no branch ($curr_branch is empty), I get the error
> message from error_on_no_merge_candidates instead of being notified that
> I am on no branch currently. Something around line 150-160 in
> git-pull.sh does not seem to work as it should.
There's no reason you couldn't pull when on no branch. It's just that,
without a branch, there's nowhere to get a default ref to merge, which
leads to having nothing to merge (if you don't give anything specific),
which leads to that error.
On the other hand, you could do:
git pull <some URL> <some branch>
and git would happily merge the specified branch of the specified
repository for you. So the reason that git-pull doesn't give you the error
you expect is that that's not necessarily an error at all.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH v2] Make git_dir a path relative to work_tree in setup_work_tree()
From: Daniel Barkalow @ 2008-06-19 18:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7vod5xuvtj.fsf@gitster.siamese.dyndns.org>
On Thu, 19 Jun 2008, Junio C Hamano wrote:
> Daniel Barkalow <barkalow@iabervon.org> writes:
>
> > diff --git a/path.c b/path.c
> > index b7c24a2..790d8d4 100644
> > --- a/path.c
> > +++ b/path.c
> > @@ -294,6 +294,23 @@ int adjust_shared_perm(const char *path)
> > /* We allow "recursive" symbolic links. Only within reason, though. */
> > #define MAXDEPTH 5
> >
> > +const char *make_relative_path(const char *abs, const char *base)
> > +{
> > + static char buf[PATH_MAX + 1];
> > + int baselen;
> > + if (!base)
> > + return abs;
>
> This special case may help the specific caller you have below, but doesn't
> it make the function do more than it advertises with its name?
I don't think so; the best path relative to nothing is the absolute path.
The idea is to return the easiest equivalent path if your pwd is known to
be "base" and you give it an absolute path. If you don't know what your
pwd is, the easiest equivalent path is the absolute path with no symlinks.
Similarly, you get an absolute path if the path you give it isn't inside
base. Maybe "make_brief_path" would be a better name?
> Other than that, I think the change is Ok, but as a "performance tweak",
> it should be backed by some numbers, please?
I was hoping Linus would provide some, since he'd noticed the slowness in
the first place. I'm not sure I have the RAM to have the kernel spend
non-trivial time looking up path elements in an all-cached tree. I'll try
to replicate Linus's test case when I get a chance if he hasn't said
anything.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables
From: Junio C Hamano @ 2008-06-19 18:40 UTC (permalink / raw)
To: Brandon Casey; +Cc: Mike Hommey, Git Mailing List
In-Reply-To: <gASAIyn4TvarEmVo9rWtEKa6eROfKXwowHxH6j05FzPARJ-CDBCHLw@cipher.nrlssc.navy.mil>
Brandon Casey <casey@nrlssc.navy.mil> writes:
> Mike Hommey wrote:
>> On Thu, Jun 19, 2008 at 12:32:02PM -0500, Brandon Casey wrote:
>>> - test_must_fail GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" git commit &&
>>> + # We intentionally do not use test_must_fail on the next line since the
>>> + # mechanism does not work when setting environment variables inline
>>> + ! GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" git commit &&
>>
>> Doesn't GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" test_must_fail git commit
>> work ?
>
> That leaves GIT_EDITOR set to the new value after the command completes.
>
> -brandon
A Subshell?
@@ -212,6 +212,7 @@ test_expect_success 'do not fire
# Must fail due to conflict
test_must_fail git cherry-pick -n master &&
echo "editor not started" >.git/result &&
- test_must_fail GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" git commit &&
+ ( GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" && export GIT_EDITOR &&
+ test_must_fail git commit ) &&
test "$(cat .git/result)" = "editor not started"
'
^ permalink raw reply
* Re: [PATCH] git-send-pack: don't consider branch lagging behind as errors.
From: Junio C Hamano @ 2008-06-19 18:33 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Jeff King, git, gitster
In-Reply-To: <20080619162801.GA2468@artemis.madism.org>
Pierre Habouzit <madcoder@debian.org> writes:
> On Thu, Jun 19, 2008 at 03:11:10PM +0000, Jeff King wrote:
>> On Thu, Jun 19, 2008 at 03:52:00PM +0200, Pierre Habouzit wrote:
>> > > - there is a possible danger with "git push -f", in that you force
>> > > both rejected branches as well as stale branches. Junio and I
>> > Well afaict this is a separate issue, as we're (with such a patch)
>> > only changing what gets printed on the console, not the internal
>> > behavior. So solving this second issue should not really be a
>> > precondition to the inclusion of such a patch.
>>
>> It is a separate issue, but it is exacerbated by hiding stale refs.
>> Imagine:
>>
>> $ git push
>> To /path/to/repo
>> ! [rejected] master -> master (non-fast forward)
>>
>> $ git push -f
>> To /path/to/repo
>> + 0abfa88...c1ed93b master -> master (forced update)
>> + 0329485...3498576 stale_branch -> stale_branch (forced update)
>>
>> I think that is a nasty surprise to spring on an unsuspecting user.
>> Another solution might be "-f" not pushing rewound branches, but then we
>> need a way to specify "no, really, push this rewound branch". Perhaps
>> "-f -f"?
>
> Well then we could keep the [stalled] lines for now until this issue
> is resolved then, despite what the people at the beginning of the other
> thread complained about. My real issue is that I have my shell
> configured so that my prompt becomes inverted if the last command
> failed. So do many people I know, and well, git push for stalled
> references should just not generate an error. _this_ is my sole concern
> :)
There are two cases the push does not fast forward. The case where you
are truly behind (aka "stale") and you and the pushed-to repository have
diverged history. Reporting success when you did not push due to the
latter is unacceptable. I personally rely on the fast-forward safety in
my push-out scripts, but I do not think it is just me. The exit status from
commands are designed to be used that way.
The issue of "many refs in the repo but I work only on a few" has already
been resolved by being able to say "I push only the current branch" in the
previous thread, I think, but I am too busy to go back to re-study the
history, so could you kindly do that for us?
The thing is, the user asked to push certain refs, and some did not get
updated. The user has the right to expect a failure indication from the
command. If you choose to _ignore_ the failure, that is _your_ choice,
like:
$ git push || :
You might argue that the case where you are truly behind _could_ be
ignored and pretend as if the user did not even _ask_ to push it (hence,
return success without doing anything to that branch), but I am not
convinced even that is a good idea.
^ permalink raw reply
* Re: [PATCH v2] Make git_dir a path relative to work_tree in setup_work_tree()
From: Junio C Hamano @ 2008-06-19 18:24 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Linus Torvalds, git
In-Reply-To: <alpine.LNX.1.00.0806182327090.19665@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> writes:
> diff --git a/path.c b/path.c
> index b7c24a2..790d8d4 100644
> --- a/path.c
> +++ b/path.c
> @@ -294,6 +294,23 @@ int adjust_shared_perm(const char *path)
> /* We allow "recursive" symbolic links. Only within reason, though. */
> #define MAXDEPTH 5
>
> +const char *make_relative_path(const char *abs, const char *base)
> +{
> + static char buf[PATH_MAX + 1];
> + int baselen;
> + if (!base)
> + return abs;
This special case may help the specific caller you have below, but doesn't
it make the function do more than it advertises with its name?
Other than that, I think the change is Ok, but as a "performance tweak",
it should be backed by some numbers, please?
> + baselen = strlen(base);
> + if (prefixcmp(abs, base))
> + return abs;
> + if (abs[baselen] == '/')
> + baselen++;
> + else if (base[baselen - 1] != '/')
> + return abs;
> + strcpy(buf, abs + baselen);
> + return buf;
> +}
> +
> const char *make_absolute_path(const char *path)
> {
> static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];
> diff --git a/setup.c b/setup.c
> index d630e37..1643ee4 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -292,7 +292,8 @@ void setup_work_tree(void)
> work_tree = get_git_work_tree();
> git_dir = get_git_dir();
> if (!is_absolute_path(git_dir))
> - set_git_dir(make_absolute_path(git_dir));
> + set_git_dir(make_relative_path(make_absolute_path(git_dir),
> + work_tree));
> if (!work_tree || chdir(work_tree))
> die("This operation must be run in a work tree");
> initialized = 1;
> --
> 1.5.4.5
^ permalink raw reply
* Re: [PATCH] Remove dependency on IO::String from Git.pm test
From: Lea Wiemann @ 2008-06-19 18:25 UTC (permalink / raw)
To: Michael Hendricks; +Cc: gitster, git
In-Reply-To: <1213796224-995-1-git-send-email-michael@ndrix.org>
Michael Hendricks wrote:
> Instead of using IO::String to create an in-memory filehandle, use
> open() with a scalar reference as the filename.
I've now sent a new version of my script that uses File::Temp and only
depends on Perl 5.6.2. Thanks for making the start!
Jakub Narebski wrote:
> [...] Lea Wiemann (who should have been CC-ed, by the way).
For the record, CC'ing me or not doesn't make a difference if it's
posted to the list (I don't actually notice whether I'm CC'ed).
-- Lea
^ permalink raw reply
* Re: difficulties with http proxy
From: J. Bruce Fields @ 2008-06-19 18:23 UTC (permalink / raw)
To: John Yesberg; +Cc: git
In-Reply-To: <1033a22d0806180206g2cf3315bh1e533902fc834ecf@mail.gmail.com>
On Wed, Jun 18, 2008 at 10:06:49AM +0100, John Yesberg wrote:
> Hi Bruce,
>
> Short version:
> 0. Using git on WinXP from
> http://msysgit.googlecode.com/files/Git-1.5.5-preview20080413.exe
> 1. When a proxy is required, the error message isn't very helpful:
> Cannot get remote repository information.
> Perhaps git-update-server-info needs to be run there?
> 2. I can't get git config to set the proxy
> 3. When I use a shell variable, I can set the proxy, but I still have
> trouble cloning git over http.
Yeah, looks like git clone could be more helpful, but unfortunately I'm
probably not the right person to help:
> John.
>
> Long version:
> I'm back behind my proxy.
> I've tried
> $ git config --global http.proxy http://proxyname:80
> and
> $ git config --global http.proxy proxyname:80
> but neither seems to work:
>
> $ git clone http://www.kernel.org/pub/scm/git/git.git git2
> Initialized empty Git repository in c:/[path]/git2/.git/
> Cannot get remote repository information.
> Perhaps git-update-server-info needs to be run there?
So would it be possible for clone to differentiate between the case
where it can't find info/refs (or whatever it's looking for there), and
the case where it just can't do any http requests at all? It's less
helpful if the "git-update-server-info needs to be run" message is the
error users always get.
>
> When I try
> $ export http_proxy=http://proxyname:80
> then the following at least starts to work:
> $ git clone http://www.kernel.org/pub/scm/git/git.git git
>
> $ git clone http://www.kernel.org/pub/scm/git/git.git git2
> Initialized empty Git repository in c:/[path]/git2/.git/
> got 7562b87f6fcfb31a5fa52d2edfe866b5f1ee08d5
> walk 7562b87f6fcfb31a5fa52d2edfe866b5f1ee08d5
> got d47c7c24b429eac4bacc107c0b5e2987db40f04a
> got 71910874a081be5196ae122c8d0b6024ec3afa5e
> walk d47c7c24b429eac4bacc107c0b5e2987db40f04a
> Getting alternates list for http://www.kernel.org/pub/scm/git/git.git
> Getting pack list for http://www.kernel.org/pub/scm/git/git.git
> Getting index for pack f7be43530c5b167d6eff8e1d4ee72d7c98aa6710
> Getting index for pack 38b949b3b9446f6ef31fe3bc5a70b09cf8cc5c82
> Getting index for pack 535070eccfdc5b080d4b38e682f66b357f3cb4bd
> got 89fa6d03bc038d6210e94d7fe9fbbffdbc84d883
> Getting index for pack fb9e204382bfea5b1821f19b7a9e82d0e5be8498
> got 02823413987d5a10364d936327e3a1cfb38cbac2
>
> [snip]
>
> Getting pack 17d608d0bea63b7c5b68bccd1515e9c5ac0961c5
> which contains 859d67990a62049c2f328dfd676cf21da8ff9a27
> got 1afd0c69ed823f419f52951c3c02dcc50f1a44d1
> got c824d887420754f98db0553a006865d31d01cf1d
> got 0b8b0ebba758871ac9c79e729664b5057128e9ac
> got 18330cdcd2cda94c9c16c02d70229fa16d4aeb33
> got d8e0a5b843d8c8d02df2150a05f4d9c5928596f4
> got e051903352f06b0ac9ba1594cd9451baf215bdc5
> error: Unable to find a2e23c928a11bf526a3c63b34c9a00a94aac6b49 under
> http://www.kernel.org/pub/scm/git/git.git
> Cannot obtain needed blob a2e23c928a11bf526a3c63b34c9a00a94aac6b49
> while processing commit 69cd7c5f72da62a221a740c4d454b29da15186f3.
I assume that could just be explained by a race where the repository got
updated at the same time you were cloning? But I wonder why this cleanup
is failing.
--b.
> rm: cannot remove directory `c:/[path]/git2/.git/clone-tmp': Directory not
> empty
> rm: cannot remove directory `c:/[path]/git2/.git': Directory not empty
> rm: cannot remove directory `c:/[path]/git2': Directory not empty
>
> If I try to clone it again, I get a similar, but not identical failure:
> ...
> Getting pack 1dbb69127c8efb2534f925b7cf031b0a2428e8b4
> which contains cb4b83de1eb48d163480185da54a828c5bb20941
> got f1875d3b3de72c1834867ac4d6afe5243f54513e
> got 347af19febc25fd47db2d11a634c2b38dbfe2c06
> got 1f405709e7341c27e20c0159fb7c17efbf85975c
> got 1df4bbf7263a22a2f8c94e0d8bd1d5fbcbab2152
> got 345943a26466dab73034b41698c54ca317cc4d75
> got b26b4e34bdfedbe7111dea48b4e5176d10555a76
> got d6c44a5ed1cd8768a30f9e0e2339bbc49e8fa57e
> error: Unable to find d69b20549bf956a5c7c3f64f5315a17ec71af038 under
> http://www.kernel.org/pub/scm/git/git.git
> Cannot obtain needed blob d69b20549bf956a5c7c3f64f5315a17ec71af038
> while processing commit d47c7c24b429eac4bacc107c0b5e2987db40f04a.
> rm: cannot remove directory `c:/[path]/git2/.git/clone-tmp': Directory not
> empty
> rm: cannot remove directory `c:/[path]/git2/.git': Directory not empty
> rm: cannot remove directory `c:/[path]/git2': Directory not empty
>
>
> On Sat, Jun 14, 2008 at 6:29 PM, J. Bruce Fields <bfields@fieldses.org>
> wrote:
>
> > On Sat, Jun 14, 2008 at 02:04:25PM +0100, John Yesberg wrote:
> > > Bruce,
> > >
> > > Sorry, missed your reply in the flood (didn't realise how busy the list
> > > would be). I'll check for more details next time I'm in the office - may
> > be
> > > Wednesday.
> > > Appreciate your concerns; I didn't want newbies (who might be at work) to
> > be
> > > turned off too early in their git experience. I'm sure we can come up
> > with
> > > something suitable.
> >
> > Sure.
> >
> > > I'll try to find out more about why the git config http.proxy isn't
> > working
> > > too.
> >
> > OK, thanks. Sorry that I don't have more constructive suggestions, but
> > I'm a bit swamped right now.... So I'll expect to hear from you next
> > week and leave thinking about this till then.
> >
> > --b.
> >
> > >
> > > John.
> > >
> > > On Wed, Jun 11, 2008 at 8:58 PM, J. Bruce Fields <bfields@fieldses.org>
> > > wrote:
> > >
> > > > On Wed, Jun 11, 2008 at 08:48:47PM +0100, John Yesberg wrote:
> > > > > ---
> > > > > Documentation/user-manual.txt | 23 +++++++++++++++++++++++
> > > > > 1 files changed, 23 insertions(+), 0 deletions(-)
> > > >
> > > > Thanks!
> > > >
> > > > >
> > > > > diff --git a/Documentation/user-manual.txt
> > > > b/Documentation/user-manual.txt
> > > > > index bfde507..02b1be0 100644
> > > > > --- a/Documentation/user-manual.txt
> > > > > +++ b/Documentation/user-manual.txt
> > > > > @@ -56,6 +56,16 @@ $ git clone
> > > > > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
> > > > > The initial clone may be time-consuming for a large project, but you
> > > > > will only need to clone once.
> > > > >
> > > > > +If there is a proxy between you and the repository you want to
> > clone,
> > > > you
> > > > > +may not be able to use the git protocol. But you can use the http
> > > > protocol, by
> > > > > +configuring the proxy. Note that the address to access the
> > repository
> > > > via http
> > > > > +may be different from the git address:
> > > > > +
> > > > > +------------------------------------------------
> > > > > +$ export http_proxy=http://theproxy.example.com:8080
> > > > > +$ git clone http://www.kernel.org/pub/scm/git/git.git
> > > > > +------------------------------------------------
> > > > > +
> > > >
> > > > Especially this early in the manual, I really want to keep the text
> > > > short--we need to get to the basics as quickly as possible--even if
> > that
> > > > means leaving out some corner cases.
> > > >
> > > > What actually happens when you run across this case as a user? Are
> > > > there any improvements to the error reporting from "clone" that would
> > > > lead the user to the right solution without needing to deal with this
> > > > case here?
> > > >
> > > > > The clone command creates a new directory named after the project
> > ("git"
> > > > > or "linux-2.6" in the examples above). After you cd into this
> > > > > directory, you will see that it contains a copy of the project
> > files,
> > > > > @@ -129,6 +139,19 @@ $ git branch
> > > > > * new
> > > > > ------------------------------------------------
> > > > >
> > > > > +It is possible, particularly on Windows platforms, that as you
> > checkout
> > > > > +the original version, it will in fact be modified, by the autocrlf
> > > > process.
> > > > > +(Windows and Unix store newlines as CRLF and LF respectively, and
> > > > autocrlf
> > > > > +tries to adapt intelligently.) If the checked out version is
> > modified,
> > > > then
> > > > > +trying to switch to a new branch will not work, because then the
> > > > uncommitted
> > > > > +changes would be lost. So you may need to add the +-f+ flag to
> > _force_
> > > > these
> > > > > +changes to be thrown away. Another option might be to edit
> > > > +~/.gitconfig+ or
> > > > > +use the following command to disable the autocrlf function.
> > > > > +
> > > > > +------------------------------------------------
> > > > > ++git config --global core.autocrlf false
> > > > > +------------------------------------------------
> > > > > +
> > > >
> > > > Again, I'd rather not deal with this case in the main text; if we
> > > > absolutely need to, a quick reference to the appropriate documentation
> > > > ("note: if you see an error like XXX, see the XXX man page...") might
> > be
> > > > the thing to do.
> > > >
> > > > --b.
> > > >
> > > > > If you decide that you'd rather see version 2.6.17, you can modify
> > > > > the current branch to point at v2.6.17 instead, with
> > > > >
> > > > > --
> > > > > 1.5.5.1015.g9d258
> > > >
> >
^ permalink raw reply
* [PATCH 2/2 v5] Git.pm: add test suite
From: Lea Wiemann @ 2008-06-19 18:18 UTC (permalink / raw)
To: git; +Cc: Lea Wiemann
In-Reply-To: <d5ac06cabb7eb235ca82525fad2e96cdab205469.1213899000.git.LeWiemann@gmail.com>
Add a shell script (t/t9700-perl-git.sh) that sets up a git repository
and a perl script (t/t9700/test.pl) that runs the actual tests.
Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
---
Changes since v4:
- Added missing ampersand (thanks Olivier!).
- Use 5.006002 (lowest possible version for Test::More).
- Use File::Temp instead of the external IO::String.
Tested with Perl 5.6, 5.8, 5.10. Diff against v4 of this patch:
index 592d79a..b2fb9ec 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -25,7 +25,7 @@ test_expect_success \
git-config --add color.test.slot1 green &&
git-config --add test.string value &&
git-config --add test.dupstring value1 &&
- git-config --add test.dupstring value2 &
+ git-config --add test.dupstring value2 &&
git-config --add test.booltrue true &&
git-config --add test.boolfalse no &&
git-config --add test.boolother other &&
diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index 8318fec..4d23125 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -1,6 +1,7 @@
#!/usr/bin/perl
use lib (split(/:/, $ENV{GITPERLLIB}));
+use 5.006002; # Test::More was introduced in 5.6.2
use warnings;
use strict;
@@ -9,7 +10,6 @@ use Test::More qw(no_plan);
use Cwd;
use File::Basename;
use File::Temp;
-use IO::String;
BEGIN { use_ok('Git') }
@@ -69,20 +69,21 @@ is($r->ident_person("Name", "email", "123 +0000"), "Name <email>",
# objects and hashes
ok(our $file1hash = $r->command_oneline('rev-parse', "HEAD:file1"), "(get file hash)");
-our $iostring = IO::String->new;
-is($r->cat_blob($file1hash, $iostring), 15, "cat_blob: size");
-is(${$iostring->string_ref}, "changed file 1\n", "cat_blob: data");
-our $tmpfile = File::Temp->new();
-print $tmpfile ${$iostring->string_ref};
+our $tmpfile = File::Temp->new;
+is($r->cat_blob($file1hash, $tmpfile), 15, "cat_blob: size");
+our $blobcontents;
+{ local $/; seek $tmpfile, 0, 0; $blobcontents = <$tmpfile>; }
+is($blobcontents, "changed file 1\n", "cat_blob: data");
+seek $tmpfile, 0, 0;
is(Git::hash_object("blob", $tmpfile), $file1hash, "hash_object: roundtrip");
$tmpfile = File::Temp->new();
print $tmpfile my $test_text = "test blob, to be inserted\n";
-$tmpfile->close;
like(our $newhash = $r->hash_and_insert_object($tmpfile), qr/[0-9a-fA-F]{40}/,
"hash_and_insert_object: returns hash");
-$iostring = IO::String->new;
-is($r->cat_blob($newhash, $iostring), length $test_text, "cat_blob: roundtrip size");
-is(${$iostring->string_ref}, $test_text, "cat_blob: roundtrip data");
+$tmpfile = File::Temp->new;
+is($r->cat_blob($newhash, $tmpfile), length $test_text, "cat_blob: roundtrip size");
+{ local $/; seek $tmpfile, 0, 0; $blobcontents = <$tmpfile>; }
+is($blobcontents, $test_text, "cat_blob: roundtrip data");
# paths
is($r->repo_path, "./.git", "repo_path");
t/t9700-perl-git.sh | 39 ++++++++++++++++++++
t/t9700/test.pl | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 139 insertions(+), 0 deletions(-)
create mode 100755 t/t9700-perl-git.sh
create mode 100755 t/t9700/test.pl
diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
new file mode 100755
index 0000000..b2fb9ec
--- /dev/null
+++ b/t/t9700-perl-git.sh
@@ -0,0 +1,39 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Lea Wiemann
+#
+
+test_description='perl interface (Git.pm)'
+. ./test-lib.sh
+
+# set up test repository
+
+test_expect_success \
+ 'set up test repository' \
+ 'echo "test file 1" > file1 &&
+ echo "test file 2" > file2 &&
+ mkdir directory1 &&
+ echo "in directory1" >> directory1/file &&
+ mkdir directory2 &&
+ echo "in directory2" >> directory2/file &&
+ git add . &&
+ git commit -m "first commit" &&
+
+ echo "changed file 1" > file1 &&
+ git commit -a -m "second commit" &&
+
+ git-config --add color.test.slot1 green &&
+ git-config --add test.string value &&
+ git-config --add test.dupstring value1 &&
+ git-config --add test.dupstring value2 &&
+ git-config --add test.booltrue true &&
+ git-config --add test.boolfalse no &&
+ git-config --add test.boolother other &&
+ git-config --add test.int 2k
+ '
+
+test_external_without_stderr \
+ 'Perl API' \
+ perl ../t9700/test.pl
+
+test_done
diff --git a/t/t9700/test.pl b/t/t9700/test.pl
new file mode 100755
index 0000000..4d23125
--- /dev/null
+++ b/t/t9700/test.pl
@@ -0,0 +1,100 @@
+#!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+use 5.006002;
+use warnings;
+use strict;
+
+use Test::More qw(no_plan);
+
+use Cwd;
+use File::Basename;
+use File::Temp;
+
+BEGIN { use_ok('Git') }
+
+# set up
+our $repo_dir = "trash directory";
+our $abs_repo_dir = Cwd->cwd;
+die "this must be run by calling the t/t97* shell script(s)\n"
+ if basename(Cwd->cwd) ne $repo_dir;
+ok(our $r = Git->repository(Directory => "."), "open repository");
+
+# config
+is($r->config("test.string"), "value", "config scalar: string");
+is_deeply([$r->config("test.dupstring")], ["value1", "value2"],
+ "config array: string");
+is($r->config("test.nonexistent"), undef, "config scalar: nonexistent");
+is_deeply([$r->config("test.nonexistent")], [], "config array: nonexistent");
+is($r->config_int("test.int"), 2048, "config_int: integer");
+is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent");
+ok($r->config_bool("test.booltrue"), "config_bool: true");
+ok(!$r->config_bool("test.boolfalse"), "config_bool: false");
+our $ansi_green = "\x1b[32m";
+is($r->get_color("color.test.slot1", "red"), $ansi_green, "get_color");
+# Cannot test $r->get_colorbool("color.foo")) because we do not
+# control whether our STDOUT is a terminal.
+
+# Failure cases for config:
+# Save and restore STDERR; we will probably extract this into a
+# "dies_ok" method and possibly move the STDERR handling to Git.pm.
+open our $tmpstderr, ">&", STDERR or die "cannot save STDERR"; close STDERR;
+eval { $r->config("test.dupstring") };
+ok($@, "config: duplicate entry in scalar context fails");
+eval { $r->config_bool("test.boolother") };
+ok($@, "config_bool: non-boolean values fail");
+open STDERR, ">&", $tmpstderr or die "cannot restore STDERR";
+
+# ident
+like($r->ident("aUthor"), qr/^A U Thor <author\@example.com> [0-9]+ \+0000$/,
+ "ident scalar: author (type)");
+like($r->ident("cOmmitter"), qr/^C O Mitter <committer\@example.com> [0-9]+ \+0000$/,
+ "ident scalar: committer (type)");
+is($r->ident("invalid"), "invalid", "ident scalar: invalid ident string (no parsing)");
+my ($name, $email, $time_tz) = $r->ident('author');
+is_deeply([$name, $email], ["A U Thor", "author\@example.com"],
+ "ident array: author");
+like($time_tz, qr/[0-9]+ \+0000/, "ident array: author");
+is_deeply([$r->ident("Name <email> 123 +0000")], ["Name", "email", "123 +0000"],
+ "ident array: ident string");
+is_deeply([$r->ident("invalid")], [], "ident array: invalid ident string");
+
+# ident_person
+is($r->ident_person("aUthor"), "A U Thor <author\@example.com>",
+ "ident_person: author (type)");
+is($r->ident_person("Name <email> 123 +0000"), "Name <email>",
+ "ident_person: ident string");
+is($r->ident_person("Name", "email", "123 +0000"), "Name <email>",
+ "ident_person: array");
+
+# objects and hashes
+ok(our $file1hash = $r->command_oneline('rev-parse', "HEAD:file1"), "(get file hash)");
+our $tmpfile = File::Temp->new;
+is($r->cat_blob($file1hash, $tmpfile), 15, "cat_blob: size");
+our $blobcontents;
+{ local $/; seek $tmpfile, 0, 0; $blobcontents = <$tmpfile>; }
+is($blobcontents, "changed file 1\n", "cat_blob: data");
+seek $tmpfile, 0, 0;
+is(Git::hash_object("blob", $tmpfile), $file1hash, "hash_object: roundtrip");
+$tmpfile = File::Temp->new();
+print $tmpfile my $test_text = "test blob, to be inserted\n";
+like(our $newhash = $r->hash_and_insert_object($tmpfile), qr/[0-9a-fA-F]{40}/,
+ "hash_and_insert_object: returns hash");
+$tmpfile = File::Temp->new;
+is($r->cat_blob($newhash, $tmpfile), length $test_text, "cat_blob: roundtrip size");
+{ local $/; seek $tmpfile, 0, 0; $blobcontents = <$tmpfile>; }
+is($blobcontents, $test_text, "cat_blob: roundtrip data");
+
+# paths
+is($r->repo_path, "./.git", "repo_path");
+is($r->wc_path, $abs_repo_dir . "/", "wc_path");
+is($r->wc_subdir, "", "wc_subdir initial");
+$r->wc_chdir("directory1");
+is($r->wc_subdir, "directory1", "wc_subdir after wc_chdir");
+TODO: {
+ local $TODO = "commands do not work after wc_chdir";
+ # Failure output is active even in non-verbose mode and thus
+ # annoying. Hence we skip these tests as long as they fail.
+ todo_skip 'config after wc_chdir', 1;
+ is($r->config("color.string"), "value", "config after wc_chdir");
+}
--
1.5.6.149.g06c04.dirty
^ permalink raw reply related
* [PATCH 1/2 v3] t/test-lib.sh: add test_external and test_external_without_stderr
From: Lea Wiemann @ 2008-06-19 18:18 UTC (permalink / raw)
To: git; +Cc: Lea Wiemann
In-Reply-To: <48596EE7.90202@free.fr>
This is for running external test scripts in other programming
languages that provide continuous output about their tests. Using
test_expect_success (like "test_expect_success 'description' 'perl
test-script.pl'") doesn't suffice here because test_expect_success
eats stdout in non-verbose mode, which is not fixable without major
file descriptor trickery.
Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
---
Olivier Marin wrote:
> Just a typo here: s/eror/error/
Thanks for spotting this, and also the missing ampersand in the other
patch!
(This typo is the only change since v2.)
t/test-lib.sh | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 58 insertions(+), 0 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 3ac8755..dc2736e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -304,6 +304,64 @@ test_expect_code () {
echo >&3 ""
}
+# test_external runs external test scripts that provide continuous
+# test output about their progress, and succeeds/fails on
+# zero/non-zero exit code. It outputs the test output on stdout even
+# in non-verbose mode, and announces the external script with "* run
+# <n>: ..." before running it. When providing relative paths, keep in
+# mind that all scripts run in "trash directory".
+# Usage: test_external description command arguments...
+# Example: test_external 'Perl API' perl ../path/to/test.pl
+test_external () {
+ test "$#" -eq 3 ||
+ error >&5 "bug in the test script: not 3 parameters to test_external"
+ descr="$1"
+ shift
+ if ! test_skip "$descr" "$@"
+ then
+ # Announce the script to reduce confusion about the
+ # test output that follows.
+ say_color "" " run $(expr "$test_count" + 1): $descr ($*)"
+ # Run command; redirect its stderr to &4 as in
+ # test_run_, but keep its stdout on our stdout even in
+ # non-verbose mode.
+ "$@" 2>&4
+ if [ "$?" = 0 ]
+ then
+ test_ok_ "$descr"
+ else
+ test_failure_ "$descr" "$@"
+ fi
+ fi
+}
+
+# Like test_external, but in addition tests that the command generated
+# no output on stderr.
+test_external_without_stderr () {
+ # The temporary file has no (and must have no) security
+ # implications.
+ tmp="$TMPDIR"; if [ -z "$tmp" ]; then tmp=/tmp; fi
+ stderr="$tmp/git-external-stderr.$$.tmp"
+ test_external "$@" 4> "$stderr"
+ [ -f "$stderr" ] || error "Internal error: $stderr disappeared."
+ descr="no stderr: $1"
+ shift
+ say >&3 "expecting no stderr from previous command"
+ if [ ! -s "$stderr" ]; then
+ rm "$stderr"
+ test_ok_ "$descr"
+ else
+ if [ "$verbose" = t ]; then
+ output=`echo; echo Stderr is:; cat "$stderr"`
+ else
+ output=
+ fi
+ # rm first in case test_failure exits.
+ rm "$stderr"
+ test_failure_ "$descr" "$@" "$output"
+ fi
+}
+
# This is not among top-level (test_expect_success | test_expect_failure)
# but is a prefix that can be used in the test script, like:
#
--
1.5.6.149.g06c04.dirty
^ permalink raw reply related
* Re: [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables
From: Brandon Casey @ 2008-06-19 18:12 UTC (permalink / raw)
To: Mike Hommey; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <frLVfPcFTL8gWg3cEzx3q-MMBWxW3Ts2EnBH2U39k5_QFQTfThtN-A@cipher.nrlssc.navy.mil>
Brandon Casey wrote:
> If I change #!/bin/sh to #!/bin/bash I get what is expected:
>
> foo: yo adrian
> foo: wo adrian
>
> #!/bin/ksh
> foo:
> foo: yo adrian
Unless I'm on solaris, then #!/bin/ksh gives me the same result
as bash:
foo: yo adrian
foo: wo adrian
-brandon
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox