Jakub Narebski [off-list ref] writes:
I have one gripe about "git-cat-file -t". I'd like it to have
-q/--quiet, -s/--silent, --hush (or --dont-spew-errors-on-stdout)
which would prohibit writing "object not found" errors on stderr
(and in gitweb case to webserver logs). I know I can use "git-cat-file -e"
to check if object exists, or modify git_get_type subroutine
# get type of given object
sub git_get_type {
my $hash = shift;
my $git_command = git_cmd_str();
open my $fd, "-|",
"$git_command cat-file -t $hash 2>/dev/null"
or return '';
It is one thing if you tend to randomly throw garbage at this
function and use it to check for object's existence, but I hope
you are already checking the user input (which is what $hash is,
I think, here), and the object is supposed to exist in the
repository you are looking at. In such a case, I think you and
your server administrator have right to know about that
situation; I do not see why you would want to squelch it.
quoted
I do not know how this %committags{} is used per project. With
a setting like repo.or.cz, it is likely that one instance of
gitweb is serving unrelated projects that have their issue
tracker at different locations using different "committags"
convention. Is the idea to eventually allow enabling/disabling
elements from the global %committags per repository somehow
(perhaps not just enable/disable but even overriding patterns or
parameters)?
I have thought about putting %committags and @committags before
loading config file
do $GITWEB_CONFIG if -e $GITWEB_CONFIG;
which can load config file depending on the project, but perhaps
it is too complicated solution.
I think you are talking about a gitweb-instance wide
customization, but that's not what I meant. I meant per-project
configuration where w/git-gui.git and w/git.git are served by
the same instance of gitweb but have pointers to different issue
trackers.
quoted
quoted
3. To not split message into many fragments we concatenate strings
if possible.
I do not know why "avoiding splits" is needed, if it raises
issues that you need to ask the list about in a message like
this...
"Avoiding splits" is needed first for performance, and second to
avoid situation where pattern would match on the boundary between
two strings in a list of tokens to process.
I wouldn't know if constantly splitting and then concatenating
is faster than just concatenatting once before output without
benchmarking, so I'd refrain from talking about performance.
Two string case may be a valid concern, though.
Dnia poniedziałek 4. grudnia 2006 11:53, Junio C Hamano napisał:
Jakub Narebski [off-list ref] writes:
quoted
I have one gripe about "git-cat-file -t". I'd like it to have
-q/--quiet, -s/--silent, --hush (or --dont-spew-errors-on-stdout)
which would prohibit writing "object not found" errors on stderr
(and in gitweb case to webserver logs). I know I can use
"git-cat-file -e" to check if object exists, or modify git_get_type
subroutine
# get type of given object
sub git_get_type {
my $hash = shift;
my $git_command = git_cmd_str();
open my $fd, "-|",
"$git_command cat-file -t $hash 2>/dev/null"
or return '';
It is one thing if you tend to randomly throw garbage at this
function and use it to check for object's existence, but I hope
you are already checking the user input (which is what $hash is,
I think, here), and the object is supposed to exist in the
repository you are looking at. In such a case, I think you and
your server administrator have right to know about that
situation; I do not see why you would want to squelch it.
I'm sorry, I should mention that this "quiet" mode of operation is
needed _only_ for committags support, for example by using
git_get_type($hash_text, -quiet=>1) in 'sha1' committag subroutine.
You might have sha1 ids in commit message which no longer point to valid
(existing) object, for example commit which is result of
"git cherry-pick -x" from no longer existing temporary branch, or commit
which is result of "git revert" on a branch which got rebased (but not
reorganized), or shortened sha1 which is no longer unique. This should
not cause errors to be written to webserver log.
By the way, is it better to use anonymous subroutines for committags
subs, or use explicit subroutines?
quoted
quoted
I do not know how this %committags{} is used per project. With
a setting like repo.or.cz, it is likely that one instance of
gitweb is serving unrelated projects that have their issue
tracker at different locations using different "committags"
convention. Is the idea to eventually allow enabling/disabling
elements from the global %committags per repository somehow
(perhaps not just enable/disable but even overriding patterns or
parameters)?
I have thought about putting %committags and @committags before
loading config file
do $GITWEB_CONFIG if -e $GITWEB_CONFIG;
which can load config file depending on the project, but perhaps
it is too complicated solution.
I think you are talking about a gitweb-instance wide
customization, but that's not what I meant. I meant per-project
configuration where w/git-gui.git and w/git.git are served by
the same instance of gitweb but have pointers to different issue
trackers.
You can always use $ENV{PATH_INFO} in $GITWEB_CONFIG value,
or check out path_info and/or $cgi->params('p') in the config file.
Or perhaps other way to set-up per repository config file.
But this is a bit complicated to set up.
I don't have definite answer about how configure committags
(both the committags enabled and sequence of committags, and
committags parameters) per repository. We can use gitweb.committags
config variable for committags enables/sequence, but how configure
committags? gitweb.committag.<name>?
BTW in some cases (e.g. xmms2 projects) issue tracker is common for
all projects hosted.
quoted
quoted
quoted
3. To not split message into many fragments we concatenate strings
if possible.
I do not know why "avoiding splits" is needed, if it raises
issues that you need to ask the list about in a message like
this...
"Avoiding splits" is needed first for performance, and second to
avoid situation where pattern would match on the boundary between
two strings in a list of tokens to process.
I wouldn't know if constantly splitting and then concatenating
is faster than just concatenatting once before output without
benchmarking, so I'd refrain from talking about performance.
Two string case may be a valid concern, though.
With current implementation it is very easy to switch this one and off.
You simply either use push, or push_or_append (or make push_or_append
do just push).
Previous (not published) version used $acc variable to concatenate
strings, but I think this solution is better (and simpler).
--
Jakub Narebski
Junio C Hamano wrote:
Jakub Narebski [off-list ref] writes:
quoted
I have thought about putting %committags and @committags before
loading config file
do $GITWEB_CONFIG if -e $GITWEB_CONFIG;
which can load config file depending on the project, but perhaps
it is too complicated solution.
I think you are talking about a gitweb-instance wide
customization, but that's not what I meant. I meant per-project
configuration where w/git-gui.git and w/git.git are served by
the same instance of gitweb but have pointers to different issue
trackers.
It looks like the hardest part with committags support wouldn't be the
actual implementation of it, but coming with easy and fast way to set
up those committags.
gitweb-xmms2 project from which the idea of committags support in gitweb
came (I think, correct me if I'm wrong) avoids this issue by having
issue tracker / bug tracker the same for all projects served by single
gitweb installation; the configuration is site-wide, and there is no
per project committags configuration.
I have imagined the following twofold solution.
1. Make it easier to have per repository gitweb configuration, for
example by having gitweb configuration file in GIT_DIR for a project,
"gitweb_conf.perl" by default:
our $GITWEB_REPO_CONFIG = $ENV{'GITWEB_REPO_CONFIG'} ||
"++GITWEB_CONFIG++";
do "$projectroot/$project/$GITWEB_REPO_CONFIG"
if -e "$projectroot/$project/$GITWEB_CONFIG";
2. Put the configuration in config file, using/like %features support.
For example gitweb.committags.<committag name> would hold parameters
for <committag>. Committags sequence would be given by sequence of
entries in config file. Comittags without options would have sole
variable entry (which I think is equivalent to being bool variable
and having 1 or 'yes' as value).
The trouble with this approach is not overriding defaults provided
while still turning on/off specific committag. And of course the fact
that for that we need rather config reader in Perl (Git.pm or
gitweb).
What do you think about it? Junio? Pasky?
--
Jakub Narebski
Jakub Narebski [off-list ref] writes:
You might have sha1 ids in commit message which no longer point to valid
(existing) object, for example commit which is result of
"git cherry-pick -x" from no longer existing temporary branch, or commit
which is result of "git revert" on a branch which got rebased (but not
reorganized), or shortened sha1 which is no longer unique. This should
not cause errors to be written to webserver log.
True.
By the way, is it better to use anonymous subroutines for committags
subs, or use explicit subroutines?
I vaguely recall a thread that discussed pros and cons of using
anonymous subroutines in certain parts of gitweb some time ago
in which even Merlyn had some comments in, but I do not recall
the technical details, sorry. My gut feeling is that the way
you illustrated your example "our %committags" definition is
fine, but it _might_ turn out that it is easier for sites or for
projects to customize their own set of rewrite rules if you had
explicitly named subroutines available. I dunno.