On Sun, 29 Jan 2012, Bernhard R. Link wrote:
> This commit changes the project listing views (project_list,
> project_index and opml) to limit the output to only projects in a
> subdirectory if the new optional parameter ?pf=directory name is used.
>
> The change is quite minimal as git_get_projects_list already can limit
> itself to a subdirectory (though that was previously only used for
> 'forks').
>
Nice and succinct.
> If there is a GITWEB_LIST file, the contents are just filtered like
> with the forks action.
>
O.K.
> Without a GITWEB_LIST file only the given subdirectory is searched
> for projects (like with forks) unless GITWEB_STRICT_EXPORT is enabled.
> In the later case GITWEB_PROJECTROOT is traversed normally (unlike
> with forks) and projects not in the directory ignored.
> (As there is no check if the filter_path would have been found in
> the usual search as the project path is checked with forks).
>
Now I understand how project_filter interacts with strict_export.
Though I am not sure if this "paranoid mode" is really necessary. I don't
see how you could get in situation where scanning from $project_list and
filtering with $project_filter prefix, and scanning from
$project_list/$project_filter would give different results.
I think you are overly paranoid here, but perhaps it is better to be
overly strict, and then relax it if it turns out to be not necessary.
> Reusing $project instead of adding a new parameter would have been
> nicer from a UI point-of-view (including PATH_INFO support) but
> would complicate the $project validating code that is currently being
> used to ensure nothing is exported that should not be viewable.
>
Sidenote: support for actionless PATH_INFO URLs would make it even more
complicated...
> Additionally change html page headers to not only link the project
> root and the currently selected project but also the directories in
> between using project_filter.
>
Excuse me changing my mind, but I think that as far as this patch series
is applied as whole, it would be better for maintability to keep those
two patches split; though put the above as a [part of] commit message
in 2/2 patch.
> Signed-off-by: Bernhard R. Link <brlink@xxxxxxxxxx>
> ---
>
> changes since v2:
> improve description
> remove || 0 for boolean argument
> merge with patch using this feature
> use user-visible configuration names instead of internal ones
>
> * Jakub Narebski <jnareb@xxxxxxxxx> [120128 23:45]:
> > "Bernhard R. Link" <brl+git@xxxxxxxxxxxxxx> writes:
> > > If strict_export is enabled and there is no projects_list, it still
> > > traverses the full tree and only filters afterwards to avoid anything
> > > getting visible by this. Otherwise only the subtree needs to be
> > > traversed, significantly reducing load times.
> > >
> > I still don't understand interaction between project_filter ('pf'),
> > $strict_export and $projects_list being either directory or a file
> > with a list of projects.
> >
> > Does it mean, that when $projects_list is a file with a list of projects,
> > and we use project_filter, then:
> >
> > * if $strict_export is false, then $project_list is ignored, and the
> > filtered list of projects is created by scanning
> > "$projectroot/$project_filter"
>
> No. If project_list is set, i.e. a file, then this is always used.
> If it is a directory (because it is not set thus set to projectroot),
> then with forks it still traverses that directory (as that was checked
> before to be a reachable project with a previous call to
> git_get_projects_list). In the case of project_filter only the directory
> is traversed without strict_export and the whole projectroot is
> traversed with strict_export.
>
O.K., now I understand it.
> Is the new description better.
>
Yes it is.
> > A few nitpicks with respect to patch itself.
> >
> > > -2827,6 +2835,7 @@ sub git_get_project_url_list {
> > >
> > > sub git_get_projects_list {
> > > my $filter = shift || '';
> > > + my $paranoid = shift || 0;
> > > my @list;
> > >
> >
> > First, undefined value is false in Perl, so there is no need for
> > " || 0" in setting $paranoid variable.
>
> I thought it make it clearer that the argument might not be set and
> what the default is. But that is personal taste.
First, optional parameter defaults to false in the 'my $foo = shift;'
or equivalent form is (I think) idiomatic Perl. Second, this way of
writing it is used through gitweb code (CodingGuidelines: imitate existing
coding practices).
> > Second, why not use global variable $strict_export instead of adding
> > another parameter to git_get_projects_list()?
>
> That would change the action=forks behaviour to traverse the whole
> projectroot two times. This way paranoia is only activated if
> strict_mode is set _and_ the argument was not yet checked to be
> reachable.
Thanks for explanation.
> gitweb/gitweb.perl | 52 ++++++++++++++++++++++++++++++++++++++++++++--------
> 1 files changed, 44 insertions(+), 8 deletions(-)
Not that large for a new feature...
--
Jakub Narebski
Poland
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html