Re: [PATCH/RFC] gitweb: Restructure projects list generation

> Jakub Narebski wrote:
> 
> > Extract filtering out forks (which is done if 'forks' feature is
> > enabled) into filter_forks_from_projects_list subroutine
> 
> Context (to remind myself): the git_get_projects_list function decides
> which repositories to show on the homepage.

> [...]
> > Both are now run _before_ displaying projects, and not while printing;
> > this allow to know upfront if there were any found projects.  Gitweb
> > now can and do print 'No such projects found' if user searches for
> > phrase which does not correspond to any project (any repository).
> 
> Aren't forks filtered out by git-get-projects-list already?

> [...]
> > Filtering out forks and marking repository (project) as having forks
> > is now consolidated into one subroutine (special case of handling
> > forks in git_get_projects_list only for $projects_list being file is
> > now removed).
> 
> Ah, no, they aren't.  That's a good change for consistency already.
> 
> > Forks handling is also cleaned up and simplified, which
> > might mean some changes in output.
> 
> (Not having read the code yet) this part is not obvious to me.  Maybe
> an example could help.

> [...]
> > The interaction between forks, content tags and searching is now made
> > more explicit: searching whether by tag, or via search form turns off
> > fork filtering (gitweb searches also forks, and will show all
> > results).
> 
> Hm, or maybe this is it.
> 
> > IMVHO the code is
> > cleaner.
> 
> So some general code tidying is intermixed.
> 
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -2646,18 +2646,20 @@ sub git_get_projects_list {
> [...]
> > -		my $dir = $projects_list . ($filter ? "/$filter" : '');
> > +		my $dir = $projects_list;
> >  		# remove the trailing "/"
> >  		$dir =~ s!/+$!!;
> > -		my $pfxlen = length("$dir");
> > -		my $pfxdepth = ($dir =~ tr!/!!);
> > +		my $pfxlen = length("$projects_list");
> > +		my $pfxdepth = ($projects_list =~ tr!/!!);
> [...]
> 
> $dir is the same as before except that trailing slashes are removed,
> and meanwhile the stripped names of directories returned from find()
> will include $filter.  Should be almost a no-op (i.e., a nice
> simplification).

> > @@ -2672,14 +2674,14 @@ sub git_get_projects_list {
> >  				# only directories can be git repositories
> >  				return unless (-d $_);
> >  				# 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) {
> 
> A bugfix.  $project_maxdepth is advertised to be relative to the
> projectroot but it was relative to $projects_list/$filter.

> [...]
> > @@ -2703,32 +2704,9 @@ sub git_get_projects_list {
> >  			if (!defined $path) {
> >  				next;
> >  			}
> > -			if ($filter ne '') {
> > -				# looking for forks;
> > -				my $pfx = substr($path, 0, length($filter));
> > -				if ($pfx ne $filter) {
> > -					next PROJECT;
> > -				}
> > -				my $sfx = substr($path, length($filter));
> > -				if ($sfx !~ /^\/.*\.git$/) {
> > -					next PROJECT;
> > -				}
> [...]
> > +			# if $filter is rpovided, check if $path begins with $filter
> 
> s/rpovided/provided/ :)

> > +			if ($filter && $path !~ m!^\Q$filter\E/!) {
> > +				next;
> 
> Why did the code check for \Q.git\E$ before?  If I'm not missing
> something, I'm happy to see the condition go.

> [...]
> > @@ -2745,6 +2721,67 @@ sub git_get_projects_list {
> >  	return @list;
> >  }
> >  
> > +# this assumes that forks follow immediately forked projects:
> > +# [ ..., 'repo.git', 'repo/fork.git', ...]; if not, set second
> > +# parameter to true value to sort projects by path first.
> > +sub filter_forks_from_projects_list {
> > +	my ($projects, $need_sorting) = @_;
> > +
> > +	@$projects = sort { $a->{'path'} cmp $b->{'path'} } @$projects
> > +		if ($need_sorting);
> 
> What happens in this scenario?
> 
> 	git.git
> 	git.related.project.git
> 	git/platforms.git

> [...]
> > @@ -4747,23 +4784,36 @@ sub fill_project_list_info {
> >  		if (!defined $pr->{'owner'}) {
> >  			$pr->{'owner'} = git_get_project_owner("$pr->{'path'}") || "";
> >  		}
> > -		if ($check_forks) {
> > -			my $pname = $pr->{'path'};
> > -			if (($pname =~ s/\.git$//) &&
> > -			    ($pname !~ /\/$/) &&
> > -			    (-d "$projectroot/$pname")) {
> > -				$pr->{'forks'} = "-d $projectroot/$pname";
> 
> So currently we treat project.git as having forks when the
> corresponding project/ directory exists, even when the latter is
> empty.  The proposed new rule is that project.git has forks if and
> only if there is at least one project/foo.git repository.  Sensible.