Re: [PATCH] gitweb: Option to omit column with time of the last change

From

Kacper Kornet <draenog@pld-linux.org>

Date

Apr 16, 2012, 10:12 UTC

Message-ID

<20120416101242.GK17753@camk.edu.pl>

In-Reply-To

<201204141516.02719.jnareb@gmail.com>

On Sat, Apr 14, 2012 at 03:16:01PM +0200, Jakub Narebski wrote:

> On Wed, 4 Apr 2012, Kacper Kornet wrote:
> > On Wed, Apr 04, 2012 at 04:31:42PM +0200, Jakub Narebski wrote:
> >> On Wed, 4 April 2012, Kacper Kornet wrote:
> >>> On Wed, Apr 04, 2012 at 01:12:01AM +0200, Jakub Narebski wrote:
> >>>> On Tue, 3 Apr 2012, Kacper Kornet wrote:

> >> Perhaps it would be better to say it like this:

> >>   $no_list_age::
> >>   	If true, omit the column with date of the most current commit on the
> >>   	projects list page. [...]

> >> It is true that it can save a bit of I/O: the git_get_last_activity()
> >> examines all branches (some of which are usually loose), and must hit
> >> the object database, unpacking/getting commit objects to get at commit
> >> date.

> >> But the fact that it also saves a fork (a git command call) per repository
> >> reminds me of something which I missed in first round of review, namely
> >> that generating 'age' and 'age_string' fields serve also as a check if
> >> repository really exist.

> >> So either we document this fact, or use some other way to verify that
> >> git repository is valid.

> > I think that git_project_list_body always works with the list returned
> > by git_get_projects_list. And git_get_projects_list validates if the
> > path is a git repository. So it should not be a problem. Please correct
> > me, if I am wrong.

> If $projects_list points to plain file, git_get_projects_list() just
> gets list of projects (and project owners) from $projects_list file
> by reading and parsing this file.  No verification that repository
> exists is done.

> If $projects_list points to directory (which is the default), then
> git_get_projects_list() scans starting from $projects_list directory
> for somtehing that _looks like_ git repository with check_head_link()
> via check_export_ok().  But you still can encounter something that
> looks like git repository (has "HEAD" file in it), but isn't.

> So we would probably want to have said variable or set of variables
> describe three states:

> * find date of last change in repository with git-for-each-ref called
>   by git_get_last_activity(), which as a side effect verifies that
>   repository actually exist.  

>   git_get_last_activity() returns empty list in list context if repo
>   does not exist, hence

>   	my (@activity) = git_get_last_activity($pr->{'path'});
>   	unless (@activity) {
>   		next PROJECT;
>   	}

> * verify that repository exists with "git rev-parse --git-dir" or
>   "git rev-parse --verify HEAD", or "git symbolic-ref HEAD", redirecting
>   stderr to /dev/null (we would probably want to save output of the
>   latter two somewhere to use it later).

>   That saves I/O, but not fork.

> * don't verify that repository exists.

> Though perhaps the last possibility isn't a good idea, so it would be
> left two-state, as in your patch. 

-- 
  Kacper Kornet