Jakub Narebski [off-list ref] writes:
Bring repository verification in check_export_ok() to standards of
is_git_directory function from setup.c (core git), and validate_headref()
to standards of the same function in path.c,... and a bit more.
validate_headref() replaces check_head_link(); note that the former
requires path to HEAD file, while the late latter path to repository.
Issues of note:
* is_git_directory() in gitweb is a bit stricter: it checks that
"/objects" and "/refs" are directories, and not only 'executable'
permission,
* validate_headref() in gitweb is a bit stricter: it checks that
reference symlink or symref points to starts with "refs/heads/",
and not only with "refs/",
* calls to check_head_link(), all of which were meant to check if
given directory can be a git repository, were replaced by newly
introduced is_git_directory().
This change is preparation for removing "Last change" column from list
of projects, which is currently used also for validating repository.
Suggested-by: Kacper Kornet <redacted>
Signed-off-by: Jakub Narebski <redacted>
---
Here is how such first step could look like...
Do you mean by "could look like" that this is still an RFC, or is this
something we want to apply and see how well it makes people's lives in
the field?
By the way, I wonder (1) if it is worth adding support for the textual
".git" file that contains "gitdir: $path", and (2) if so how big a
change would we need to do so.
quoted hunk
gitweb/gitweb.perl | 52 ++++++++++++++++++++++++++++++++++++++++++----------
1 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 098e527..767d7a5 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -621,19 +621,51 @@ sub feature_avatar {
return @val ? @val : @_;
}
-# checking HEAD file with -e is fragile if the repository was
-# initialized long time ago (i.e. symlink HEAD) and was pack-ref'ed
-# and then pruned.
-sub check_head_link {
- my ($dir) = @_;
- my $headfile = "$dir/HEAD";
- return ((-e $headfile) ||
- (-l $headfile && readlink($headfile) =~ /^refs\/heads\//));
+# Test if it looks like we're at a git directory.
+# We want to see:
+#
+# - an objects/ directory,
+# - a refs/ directory,
+# - either a HEAD symlink or a HEAD file that is formatted as
+# a proper "ref:", or a regular file HEAD that has a properly
+# formatted sha1 object name.
+#
+# See is_git_directory() in setup.c
+sub is_git_directory {
+ my $dir = shift;
+ return
+ -x "$dir/objects" && -d _ &&
+ -x "$dir/refs" && -d _ &&
+ validate_headref("$dir/HEAD");
+}
+
+# Check HEAD file, that it is either
+#
+# - a "refs/heads/.." symlink, or
+# - a symbolic ref to "refs/heads/..", or
+# - a detached HEAD.
+#
+# See validate_headref() in path.c
+sub validate_headref {
+ my $headfile = shift;
+ if (-l $headfile) {
+ return readlink($headfile) =~ m!^refs/heads/!;
+
+ } elsif (-e _) {
+ open my $fh, '<', $headfile or return;
+ my $line = <$fh>;
+ close $fh or return;
+
+ return
+ $line =~ m!^ref:\s*refs/heads/! || # symref
+ $line =~ m!^[0-9a-z]{40}$!i; # detached HEAD
+ }
+ return;
}
sub check_export_ok {
my ($dir) = @_;
- return (check_head_link($dir) &&
+ return (is_git_directory($dir) &&
(!$export_ok || -e "$dir/$export_ok") &&
(!$export_auth_hook || $export_auth_hook->($dir)));
}@@ -842,7 +874,7 @@ sub evaluate_path_info {
# find which part of PATH_INFO is project
my $project = $path_info;
$project =~ s,/+$,,;
- while ($project && !check_head_link("$projectroot/$project")) {
+ while ($project && !is_git_directory("$projectroot/$project")) {
$project =~ s,/*[^/]*$,,;
}
return unless $project;
On Thu, 19 April 2012, Junio C Hamano wrote:
Jakub Narebski [off-list ref] writes:
quoted
Bring repository verification in check_export_ok() to standards of
is_git_directory function from setup.c (core git), and validate_headref()
to standards of the same function in path.c,... and a bit more.
validate_headref() replaces check_head_link(); note that the former
requires path to HEAD file, while the late latter path to repository.
Issues of note:
* is_git_directory() in gitweb is a bit stricter: it checks that
"/objects" and "/refs" are directories, and not only 'executable'
permission,
* validate_headref() in gitweb is a bit stricter: it checks that
reference symlink or symref points to starts with "refs/heads/",
and not only with "refs/",
* calls to check_head_link(), all of which were meant to check if
given directory can be a git repository, were replaced by newly
introduced is_git_directory().
This change is preparation for removing "Last change" column from list
of projects, which is currently used also for validating repository.
Suggested-by: Kacper Kornet <redacted>
Signed-off-by: Jakub Narebski <redacted>
---
Here is how such first step could look like...
Do you mean by "could look like" that this is still an RFC, or is this
something we want to apply and see how well it makes people's lives in
the field?
"Here is how such first step could look like" was directed to Kacper... :-)
Kacper Kornet (who started this thread with "[PATCH] gitweb: Option
to omit column with time of the last change") wants to have an option
to remove "Last Change" column from projects list page, and "Owner"
column and field from all gitweb views. This will allow to generate
projects list page with 1 call to git command rather than 2*N+1, where
N is number of repositories shown...
...but we use the fact that "git --git-dir=$GIT_DIR for-each-ref ..."
succeed or fails to verify that given path points to git repository.
That is why I proposed this commit to be first patch in hopefully
upcoming Kacper's new version of patch series.
But in current gitweb (without Kacper's planned patches) this change
doesn't bring much, as git repositories are verified outside of
is_git_directory() check... well, perhaps with exception of possible
corner case when one is using path_info gitweb URL...
By the way, I wonder (1) if it is worth adding support for the textual
".git" file that contains "gitdir: $path", and (2) if so how big a
change would we need to do so.
I don't think that it would be big changeto add support for "gitlink"
files, assuming that 'git --git-dir=<gitlink file> ...' works correctly.
I would put that addition in a separate commit, though.
BTW. does core git limit number of redirections, or have some loop
detection?
--
Jakub Narebski
Poland
On Thu, 19 April 2012, Jakub Narebski wrote:
On Thu, 19 April 2012, Junio C Hamano wrote:
quoted
By the way, I wonder (1) if it is worth adding support for the textual
".git" file that contains "gitdir: $path", and (2) if so how big a
change would we need to do so.
I don't think that it would be big change to add support for "gitlink"
files, assuming that 'git --git-dir=<gitlink file> ...' works correctly.
I would put that addition in a separate commit, though.
Well, I actually tried to write such commit, adding support for
'gitlink' files, and it turned out to be harder than I thought.
The problem that stumped me for now is that gitweb tries to read
many files inside git repository ('export-ok', 'description',
'cloneurl', 'category', etc.), allof which must be redirected to
real git directory.
I still think it is doable, but I wonder if it is worth it...
Below there is work in progress patch, which doesn't use resolve_gitdir
yet, and without any tests.
-- >8 ---------- >8 --
Subject: [PATCH] gitweb: Add support for "gitdir: <path>" gitfile
Suggested-by: Junio C Hamano <redacted>
Signed-off-by: Jakub Narebski <redacted>
---
gitweb/gitweb.perl | 54 ++++++++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 767d7a5..8d70a0a 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -639,6 +639,52 @@ sub is_git_directory {
validate_headref("$dir/HEAD");
}
+# Try to read the location of the git directory from the .git file,
+# return path to git directory if found.
+#
+# See read_gitfile in setup.c
+sub read_gitfile {
+ my $path = shift;
+ # note: the "basename eq '.git'" check isn't in setup.c version
+ return unless ($path =~ m!(?:^|/)\.git/*$! && -f $path);
+
+ open my $fh, '<', $path or return;
+ my $contents = do { local $/ = undef; <$fh> };
+ close $fh or return;
+ return unless defined $contents;
+ chomp($contents);
+
+ return unless ($contents =~ s!^gitdir: !!);
+
+ if (!File::Spec->file_name_is_absolute($contents)) {
+ $contents = File::Spec->catfile(File::Basename::dirname($path), $contents);
+ }
+
+ return unless is_git_directory($contents);
+ return $contents;
+}
+
+# Test if it looks like we're at a git repository
+#
+# - a '.git' file containing "gitdir: <path>",
+# - a git directory.
+sub is_git_repository {
+ my $path = shift;
+ return defined(read_gitfile($path)) || is_git_directory($path);
+}
+
+# Return directory of a git repository, resolving '.git' files
+# (file containing "gitdir: <path>") if any
+#
+# See resolve_gitdir in setup.c
+sub resolve_gitdir {
+ my $suspect = shift;
+ if (is_git_directory($suspect)) {
+ return $suspect;
+ }
+ return read_gitfile($suspect);
+}
+
# Check HEAD file, that it is either
#
# - a "refs/heads/.." symlink, or@@ -665,7 +711,7 @@ sub validate_headref {
sub check_export_ok {
my ($dir) = @_;
- return (is_git_directory($dir) &&
+ return (is_git_repository($dir) &&
(!$export_ok || -e "$dir/$export_ok") &&
(!$export_auth_hook || $export_auth_hook->($dir)));
}@@ -874,7 +920,7 @@ sub evaluate_path_info {
# find which part of PATH_INFO is project
my $project = $path_info;
$project =~ s,/+$,,;
- while ($project && !is_git_directory("$projectroot/$project")) {
+ while ($project && !is_git_repository("$projectroot/$project")) {
$project =~ s,/*[^/]*$,,;
}
return unless $project;@@ -3094,8 +3140,8 @@ sub git_get_projects_list {
our $projectroot;
# skip project-list toplevel, if we get it.
return if (m!^[/.]$!);
- # only directories can be git repositories
- return unless (-d $_);
+ # only directories or gitlink files can be git repositories
+ return unless (-d $_ || (-f _ && $File::Find::name =~ m!(?:^|/)\.git!));
# don't traverse too deep (Find is super slow on os x)
# $project_maxdepth excludes depth of $projectroot
if (($File::Find::name =~ tr!/!!) - $pfxdepth > $project_maxdepth) {--
1.7.9