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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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.

I think that even in this case check_export_ok is called. But there is
still the problem you have mentioned below.

> 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. 

My tests show that forks are also a bottleneck in my setup. On the other
hand I think that I can trust that my projects.list contains only valid
repositories. So I would prefer to have a don't verify option. Or there
is a possibility to write perl function with the same functionality as
is_git_directory() from setup.c and use it to verify if the directory is a
valid git repo.

  Kacper Kornet
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at

[Newbies FAQ]     [Linux Kernel Development]     [Free Online Dating]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Free Online Dating]     [Photo]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]     [Linux Resources]

Add to Google