The current implementation only hyperlinks the first hash on
a given line of the commit message. It seems sensible to
highlight all of them if there are multiple, and it seems
plausible that there would be multiple even with a tidy line
length limit, because they can be abbreviated as short as 8
characters.
Benchmark:
I wanted to make sure that using the 'e' switch to the Perl regex
wasn't going to kill performance, since this is called once per commit
message line displayed.
In all three A/B scenarios I tried, the A and B yielded the same
results within 2%, where A is the version of code before this patch
and B is the version after.
1: View a commit message containing the last 1000 commit hashes
2: View a commit message containing 1000 lines of 40 dots to avoid
hyperlinking at the same message length
3: View a short merge commit message with a few lines of text and
no hashes
All were run in CGI mode on my sub-production hardware on a recent
clone of git.git. Numbers are the average of 10 reqests per second
with the first request discarded, since I expect this change to affect
primarily CPU usage. Measured with ApacheBench.
Note that the web page rendered was the same; while the new code
supports multiple hashes per line, there was at most one per line.
The primary purpose of scenarios 2 and 3 were to verify that the
addition of 1000 commit messages had an impact on how much of the time
was spent rendering commit messages. They were all within 2% of 0.80
requests per second (much faster).
So I think the patch has no noticeable effect on performance.
Signed-off-by: Marcel M. Cary <marcel@oak.homeunix.org>
Acked-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a feature like "blame" is permitted to be overridden in the
repository configuration but it is not actually set in the repository,
a warning is emitted due to the undefined value of the repository
configuration, even though it's a perfectly normal condition.
Emitting warning is grounds for test failure in the gitweb test
script.
This error was caused by rewrite of git_get_project_config from using
"git config [<type>] <name>" for each individual configuration
variable checked to parsing "git config --list --null" output in
commit b201927 (gitweb: Read repo config using 'git config -z -l').
Earlier version of git_get_project_config was returning empty string
if variable do not exist in config; newer version is meant to return
undef in this case, therefore change in feature_bool was needed.
Additionally config_to_* subroutines were meant to be invoked only if
configuration variable exists; therefore we added early return to
git_get_project_config: it now returns no value if variable does not
exists in config. Otherwise config_to_* subroutines (config_to_bool
in paryicular) wouldn't be able to distinguish between the case where
variable does not exist and the case where variable doesn't have value
(the "[section] noval" case, which evaluates to true for boolean).
While at it fix bug in config_to_bool, where checking if $val is
defined (if config variable has value) was done _after_ stripping
leading and trailing whitespace, which lead to 'Use of uninitialized
value' warning.
Add test case for features overridable but not overriden in repo
config, and case for no value boolean configuration variable.
Signed-off-by: Marcel M. Cary <marcel@oak.homeunix.org>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
CGI::url() has some issues when rebuilding the script URL if the script
is a DirectoryIndex.
One of these issue is the inability to strip PATH_INFO, which is why
we had to do it ourselves.
Another issue is that the resulting URL cannot be used for the <base>
tag: it works if we're the DirectoryIndex at the root level, but not
otherwise.
We fix this by building the proper base URL ourselves, and improve the
comment about the need to strip PATH_INFO manually while we're at it.
Additionally t/t9500-gitweb-standalone-no-errors.sh had to be modified
to set SCRIPT_NAME variable (CGI standard states that it MUST be set,
and now gitweb uses it if PATH_INFO is not empty, as is the case for
some of tests in t9500).
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a gitweb configuration variable $prevent_xss that disables features
to prevent content in repositories from launching cross-site scripting
(XSS) attacks in the gitweb domain. Currently, this option makes gitweb
ignore README.html (a better solution may be worked out in the future)
and serve a blob_plain file of an untrusted type with
"Content-Disposition: attachment", which tells the browser not to show
the file at its original URL.
The XSS prevention is currently off by default.
Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make SHA-1 regexp to be turned into hyperlink (the SHA-1 committag)
to match word boundary at the beginning and the end. This way we
reduce number of false matches, for example we now don't match
0x74a5cd01 which is hex decimal (for example memory address),
but is not SHA-1.
Suggested-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Acked-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Gitweb links to a number of static files such as CSS stylesheets,
favicon or the git logo. When, such as with the default Makefile, the
paths to these files are relative (i.e. doesn't start with a "/"), the
files become inaccessible in any view other tha project list and summary
page if gitweb is invoked with a non-empty PATH_INFO.
Fix this by adding a <base> element pointing to the script's own URL,
which ensure that all relative paths will be resolved correctly.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Acked-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Offering Last-modified header for feeds is only half the work, even if
we bail out early on HEAD requests. We should also check that same date
against If-modified-since, and bail out early with 304 Not Modified if
that's the case.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The last-modified time header added by RSS to increase cache hits from
readers should be set to the date the repository was last modified. The
author time in this respect is not a good guess because the last commit
might come from a oldish patch.
Use the committer time for the last-modified header to ensure a more
correct guess of the last time the repository was modified.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The RSS 2.0 specifications defines not one but _two_ dates for its
channel element! Woohoo! Luckily, it seems that consensus seems to be
that if both are present they should be equal, except for some very
obscure and discouraged cases. Since lastBuildDate would make more sense
for us and pubDate seems to be the most commonly used, we defined both
and make them equal.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The RSS 2.0 specification allows an optional managingEditor tag for the
channel, containing the "email address for person responsible for editorial
content", which is basically the project owner.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add <generator> tag to RSS and Atom feed. Versioning info (gitweb/git
core versions, separated by a literal slash) is stored in the
appropriate attribute for the Atom feed, and in the tag content for the
RSS feed.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Define the channel image for the rss feed when the logo or favicon are
defined, preferring the former to the latter. As suggested in the RSS
2.0 specifications, the image's title and link as set to the same as the
channel's.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Suggest opml.xml as name for OPML view by providing the appropriate
header, consistently with similar usage in project_index view.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Acked-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* jn/gitweb-blame:
gitweb: cache $parent_commit info in git_blame()
gitweb: A bit of code cleanup in git_blame()
gitweb: Move 'lineno' id from link to row element in git_blame
With PATH_INFO urls, actions for the projects list (e.g. opml,
project_index) were being put in the URL right after the base. The
resulting URL is not properly parsed by gitweb itself, since it expects
a project name as first component of the URL.
Accepting global actions in use_pathinfo is not a very robust solution
due to possible present and future conflicts between project names and
global actions, therefore we just refuse to create PATH_INFO URLs when
the project is not defined.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Acked-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since the OPML project list view was hand-coding the RSS and HTML URLs,
it didn't respect global options such as use_pathinfo. Make it use
href() to ensure consistency with the rest of the gitweb setup.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Acked-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When $filter was empty, the path passed to check_export_ok would
contain an extra '/', which some implementations of export_auth_hook
are sensitive to.
It makes more sense to fix this here than to handle the special case
in each implementation of export_auth_hook.
Signed-off-by: Devin Doucette <devin@doucette.cc>
Acked-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We link to patch view in commit and commitdiff view, and to patches view
in log and shortlog view.
In (short)log view, the link is only offered when the number of commits
shown is no more than the allowed maximum number of patches.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The only difference between patch and patches view is in the treatement
of single commits: the former only displays a single patch, whereas the
latter displays a patchset leading to the specified commit.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since we are going to introduce an additional parameter for
git_commitdiff to tune patch view, we switch to named/hash-based
parameter passing for clarity and robustness.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The output of commitdiff_plain is not intended for git-am:
* when given a range of commits, commitdiff_plain publishes a single
patch with the message from the first commit, instead of a patchset
* the hand-built email format replicates the commit summary both as
email subject and as first line of the email itself, resulting in
a duplication if the output is used with git-am.
We thus create a new view that can be fed to git-am directly, allowing
patch exchange via gitweb. The new view exposes the output of git
format-patch directly, limiting it to a single patch in the case of a
single commit.
A configurable upper limit defaulting to 16 is imposed on the number of
commits which will be included in a patchset, to prevent DoS attacks on
the server. Setting the limit to 0 will disable the patch view, setting
it to a negative number will remove the limit.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jakub says that legacy-style URI to view two blob differences are never
generated since 1.4.3. This codepath runs "git diff" Porcelain from the
gitweb, which is a no-no.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The boolean feature subroutines behaved identically except for the
name of the configuration option, so make that a parameter and unify
them.
Signed-off-by: Matt Kraai <kraai@ftbfs.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Luben Tuikov changed 'lineno' link from leading to commit which gave
current version of given block of lines, to leading to parent of this
commit in 244a70e (Blame "linenr" link jumps to previous state at
"orig_lineno"). This made possible data mining using 'blame' view.
The current implementation calls rev-parse once per each blamed line
to find parent revision of blamed commit, even when the same commit
appears more than once, which is inefficient.
This patch mitigates this issue by caching $parent_commit info in
%metainfo, which makes gitweb call rev-parse only once per each
unique commit in the output from "git blame".
In the tables below you can see simple benchmark comparing gitweb
performance before and after this patch
File | L[1] | C[2] || Time0[3] | Before[4] | After[4]
====================================================================
blob.h | 18 | 4 || 0m1.727s | 0m2.545s | 0m2.474s
GIT-VERSION-GEN | 42 | 13 || 0m2.165s | 0m2.448s | 0m2.071s
README | 46 | 6 || 0m1.593s | 0m2.727s | 0m2.242s
revision.c | 1923 | 121 || 0m2.357s | 0m30.365s | 0m7.028s
gitweb/gitweb.perl | 6291 | 428 || 0m8.080s | 1m37.244s | 0m20.627s
File | L/C | Before/After
=========================================
blob.h | 4.5 | 1.03
GIT-VERSION-GEN | 3.2 | 1.18
README | 7.7 | 1.22
revision.c | 15.9 | 4.32
gitweb/gitweb.perl | 14.7 | 4.71
As you can see the greater ratio of lines in file to unique commits
in blame output, the greater gain from the new implementation.
Legend:
[1] Number of lines:
$ wc -l <file>
[2] Number of unique commits in the blame output:
$ git blame -p <file> | grep author-time | wc -l
[3] Time for running "git blame -p" (user time, single run):
$ time git blame -p <file> >/dev/null
[4] Time to run gitweb as Perl script from command line:
$ gitweb-run.sh "p=.git;a=blame;f=<file>" > /dev/null 2>&1
The gitweb-run.sh script includes slightly modified (with adjusted
pathnames) code from gitweb_run() function from the test script
t/t9500-gitweb-standalone-no-errors.sh; gitweb config file
gitweb_config.perl contents (again up to adjusting pathnames; in
particular $projectroot variable should point to top directory of git
repository) can be found in the same place.
Discussion
~~~~~~~~~~
A possible future improvement would be to open a bidi pipe to
"git cat-file --batch-check", (like in Git::Repo in gitweb caching by
Lea Wiemann), feed $long_rev^ to it, and parse its output, which is
in the following form:
926b07e694599d86cec668475071b32147c95034 commit 637
This would mean one call to git-cat-file for the whole 'blame' view,
instead of one call to git-rev-parse per each unique commit in blame
output.
Yet another solution would be to change use of validate_refname() to
validate_revision() when checking script parameters (CGI query or
path_info), with validate_revision being something like the following:
sub validate_revision {
my $rev = shift;
return validate_refname(strip_rev_suffixes($rev));
}
so we don't need to calculate $long_rev^, but can pass "$long_rev^" as
'hb' parameter.
This solution has the advantage that it can be easily adapted to future
incremental blame output.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Acked-by: Luben Tuikov <ltuikov@yahoo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Among others, here are the highlights:
* move variable declaration closer to the place it is set and used,
if possible,
* uniquify and simplify coding style a bit, which includes removing
unnecessary '()'.
* check type only if $hash was defined, as otherwise from the way
git_get_hash_by_path() is called (and works), we know that it is
a blob,
* use modern calling convention for git-blame,
* remove unused variable,
* don't use implicit variables ($_),
* add some comments
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Acked-by: Luben Tuikov <ltuikov@yahoo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move l<line number> ID from <a> link element inside table row (inside
cell element for column with line numbers), to encompassing <tr> table
row element. It was done to make it easier to manipulate result HTML
with DOM, and to be able write 'blame_incremental' view with the same,
or nearly the same result.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Acked-by: Luben Tuikov <ltuikov@yahoo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In insert_file() subroutine (which is used to insert HTML fragments as
custom header, footer, hometext (for projects list view), and per
project README.html (for summary view)) we used:
map(to_utf8, <$fd>);
This doesn't work, and other form has to be used:
map { to_utf8($_) } <$fd>;
Now with test for t9600 added, for $GIT_DIR/README.html.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* maint:
GIT 1.6.0.5
"git diff <tree>{3,}": do not reverse order of arguments
tag: delete TAG_EDITMSG only on successful tag
gitweb: Make project specific override for 'grep' feature work
http.c: use 'git_config_string' to get 'curl_http_proxy'
fetch-pack: Avoid memcpy() with src==dst
The 'grep' feature was marked in the comments as having project
specific config, but it lacked 'sub' key required for it to work.
Kind-of-Noticed-by: Matt Kraai <kraai@ftbfs.org>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use new insert_file() subroutine to insert HTML chunks from external
files: $site_header, $home_text (by default indextext.html),
$site_footer, and $projectroot/$project/REAME.html.
All non-ASCII chars of those files will be broken by Perl IO layer
without decoding to utf8, so insert_file() does to_utf8() on each
printed line; alternate solution would be to open those files with
"binmode $fh, ':utf8'", or even all files with "use open qw(:std :utf8)".
Note that inserting README.html lost one of checks for simplicity.
Noticed-by: Tatsuki Sugiura <sugi@nemui.org>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is taken from a patch from Giuseppe but unfortunately it came
too late to replace the series that was already on "next". The comment
he updated here is better than the version we had previously, so I am
cherry-picking this bit not to lose it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The gitweb_get_feature() function retrieves the configuration parameters
for the feature (such as the list of snapshot formats or the list of
additional actions), but it is very often used to see if feature is
enabled (which is returned as the first element in the list).
Because accepting the returned list in the scalar context by mistake
yields the number of elements in the array, which is non-zero in all
cases, such a mistake would result in a bug for the latter use, with
disabled features appearing enabled. All existing callers that call the
function for this purpose assign the return value in the list context to
retrieve the first element, but that is only because we fixed careless
callers recently.
This adds gitweb_check_feature() as a wrapper to gitweb_get_feature() that
can be called safely in the scalar context to see if a feature is enabled
to reduce the risk of future bugs. Callers of "get" that use the call
only to see if the feature is enabled are updated to call this wrapper.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function is about retrieving the configuration parameter list for the
feature. A more robust way to check if a feature is enabled will be
introduced in the next patch, and the function will be called
gitweb_check_feature.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
gitweb_check_feature() function is to retrieve the configuration parameter
list and calling it in the scalar context does not give its first element
that tells if the feature is enabled. This fixes all the existing callers
to call the function correctly in the list context.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* gb/gitweb-snapshot-pathinfo:
gitweb: embed snapshot format parameter in PATH_INFO
gitweb: retrieve snapshot format from PATH_INFO
gitweb: make the supported snapshot formats array global
ModPerl::Registry precompiles scripts by wrapping them
in a subroutine. This causes ordinary subroutines of the
script to become nested, and warnings appear:
gitweb.cgi: Variable "$path_info" will not stay shared
This warning means that $path_info was declared as 'my',
and thus according to the perl evaluation rules all nested
subroutines will retain a reference to the instance of the
variable used in the first invocation of the master script.
When the script (i.e. the master meta-subroutine) is executed
the second time, it will use a new instance, so the logic
breaks. To avoid this it is necessary to declare all global
variables as 'our', which places them at the package level.
Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
Acked-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a configuration variable that can be used to specify an
arbitrary subroutine that will be called in the same situations
where $export_ok is checked, and its return value used
to decide whether the repository is to be shown.
This allows the user to implement custom authentication
schemes, for example by issuing a subrequest through mod_perl
and checking if Apache will authorize it.
Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
Acked-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
GitWeb source contains a special function that implements the
export_ok check, but validate_project still uses a separate copy
of essentially the same code.
This patch makes it use the dedicated function, thus ensuring
that all checks are done through a single code path.
Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
Acked-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When PATH_INFO is active, get rid of the sf CGI parameter by embedding
the snapshot format information in the PATH_INFO URL, in the form of an
appropriate extension.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We parse requests for $project/snapshot/$head.$sfx as equivalent to
$project/snapshot/$head?sf=$sfx, where $sfx is any of the known
(although not necessarily supported) snapshot formats (or its default
suffix).
The filename for the resulting package preserves the requested
extensions (so asking for a .tgz gives a .tgz, and asking for a .tar.gz
gives a .tar.gz), although for obvious reasons it doesn't preserve the
basename (git/snapshot/next.tgz returns a file names git-next.tgz).
This introduces a potential case for ambiguity if a project has a head
that ends with a snapshot-like suffix (.zip, .tgz, .tar.gz, etc) and the
sf CGI parameter is not present; however, gitweb only produces URLs with
the sf parameter currently, so this is only a potential issue for
hand-coded URLs for extremely unusual project.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The array of supported snapshot format is used and defined (with two
different names) in two routines, one of which (format_snapshot_links)
is often called multiple times per page.
Simplify code and speed up page generation by making the array global.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
