Re: [PATCH] gitweb: Use File::Find::find in git_get_projects_list

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

 



Jakub Narebski <jnareb@xxxxxxxxx> writes:

> +		sub wanted {
> +			# skip dot files (hidden files), check only directories
> +			#return if (/^\./);

Leftover comment?

> +			my $subdir = substr($File::Find::name, $pfxlen + 1);
> +			# we check related file in $projectroot
> +			if (-e "$projectroot/$subdir/HEAD") {
> +				push @list, { path => $subdir };
> +				$File::Find::prune = 1;

We might want to do an extra cheap check to make what we found
is sane, to prevent us getting confused by a random file whose
name happens to be HEAD.

For example, it is a regular file whose contents is a single
line and begins with "ref: refs/heads/" (16 bytes) or it is a
symlink and readlink result begins with "refs/heads/" (11
bytes).

If you feel opening and reading the file is an added overhead,
checking for $project/$subdir/{objects,refs}/ directories might
be a good alternative.

> +		File::Find::find({
> +			no_chdir => 1, # do not change directory
> +			follow_fast => 1, # follow symbolic links

What is the reason behind choosing follow_fast?  By saying
follow_anything, you choose to care about cases where there are
symlinks under projectroot to point at various projects.  If
that is the case, don't you want to make sure you include the
same project only once?

> +			#follow_skip => 2, # ignore duplicated files and directories

Leftover comment?

About these two leftover comments, if you decided you did not
want them, please do not leave them behind.

If on the other hand you wanted to hint others that you are not
sure about your decision, it would probably be better to say
that honestly in the comment, perhaps mark the message as RFC,
and describe what the issues are, like so:

	sub wanted {
		# We might want to also ignore dot files, by
                # saying "return if /^\./;" here, but there is
                # no inherent reason for us to forbid a repository
                # name from starting with a dot.

                # We check only if a directory looks like a git
                # repo and do not care about non directories.
                # Note that this cannot be done with "-d _"
                # because we are using follow_fast and the last
                # stat was done with lstat(); we want to catch a
                # symlink that points at a directory.
                return unless -d $_;
                ...

-
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]