* Re: [PATCHv2 0/2] fixes to gitweb feature check code
From: Giuseppe Bilotta @ 2008-11-29 11:13 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Petr Baudis, Junio C Hamano
In-Reply-To: <200811291148.15182.jnareb@gmail.com>
2008/11/29 Jakub Narebski <jnareb@gmail.com>:
> Dnia piątek 28. listopada 2008 21:39, Giuseppe Bilotta napisał:
>> The first patch introduces git_get_feature() to clearly distinguish the
>> feature retrieval from the boolean feature check (which is kept at
>> git_check_feature()). The new function is used where appropriate.
> It is gitweb_get_feature() (for retrieval) and gitweb_check_feature()
> (for checking), not git_get_feature() and git_check_feature().
Ahem. Right. I'll resend the second patch with a fixed subject and
commit message 8-P
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* [PATCHv2 2/2] gitweb: clean up gitweb_check_feature() calls
From: Giuseppe Bilotta @ 2008-11-29 11:15 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Giuseppe Bilotta
In-Reply-To: <1227904793-1821-3-git-send-email-giuseppe.bilotta@gmail.com>
Since git_check_feature now returns a scalar, the parentheses around the
variables initialized from gitweb_check_feature() are not needed.
We remove them for stylistic consistency and to prevent unnecessary
ambiguity in the code.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
Fixed the commit message, use this instead of the previous one.
gitweb/gitweb.perl | 26 +++++++++++++-------------
1 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 128d7ad..b0d00ea 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -828,7 +828,7 @@ sub href (%) {
}
}
- my ($use_pathinfo) = gitweb_check_feature('pathinfo');
+ my $use_pathinfo = gitweb_check_feature('pathinfo');
if ($use_pathinfo) {
# try to put as many parameters as possible in PATH_INFO:
# - project name
@@ -2119,7 +2119,7 @@ sub git_get_projects_list {
$filter ||= '';
$filter =~ s/\.git$//;
- my ($check_forks) = gitweb_check_feature('forks');
+ my $check_forks = gitweb_check_feature('forks');
if (-d $projects_list) {
# search in directory
@@ -2965,7 +2965,7 @@ EOF
}
print "</div>\n";
- my ($have_search) = gitweb_check_feature('search');
+ my $have_search = gitweb_check_feature('search');
if (defined $project && $have_search) {
if (!defined $searchtext) {
$searchtext = "";
@@ -2979,7 +2979,7 @@ EOF
$search_hash = "HEAD";
}
my $action = $my_uri;
- my ($use_pathinfo) = gitweb_check_feature('pathinfo');
+ my $use_pathinfo = gitweb_check_feature('pathinfo');
if ($use_pathinfo) {
$action .= "/".esc_url($project);
}
@@ -3472,7 +3472,7 @@ sub is_patch_split {
sub git_difftree_body {
my ($difftree, $hash, @parents) = @_;
my ($parent) = $parents[0];
- my ($have_blame) = gitweb_check_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
print "<div class=\"list_head\">\n";
if ($#{$difftree} > 10) {
print(($#{$difftree} + 1) . " files changed:\n");
@@ -3986,7 +3986,7 @@ sub git_project_list_body {
# actually uses global variable $project
my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
- my ($check_forks) = gitweb_check_feature('forks');
+ my $check_forks = gitweb_check_feature('forks');
my @projects = fill_project_list_info($projlist, $check_forks);
$order ||= $default_projects_order;
@@ -4446,7 +4446,7 @@ sub git_summary {
my @taglist = git_get_tags_list(16);
my @headlist = git_get_heads_list(16);
my @forklist;
- my ($check_forks) = gitweb_check_feature('forks');
+ my $check_forks = gitweb_check_feature('forks');
if ($check_forks) {
@forklist = git_get_projects_list($project);
@@ -4475,7 +4475,7 @@ sub git_summary {
}
# Tag cloud
- my $show_ctags = (gitweb_check_feature('ctags'))[0];
+ my $show_ctags = gitweb_check_feature('ctags');
if ($show_ctags) {
my $ctags = git_get_project_ctags($project);
my $cloud = git_populate_project_tagcloud($ctags);
@@ -4765,7 +4765,7 @@ sub git_blob {
$expires = "+1d";
}
- my ($have_blame) = gitweb_check_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
open my $fd, "-|", git_cmd(), "cat-file", "blob", $hash
or die_error(500, "Couldn't cat $file_name, $hash");
my $mimetype = blob_mimetype($fd, $file_name);
@@ -4858,7 +4858,7 @@ sub git_tree {
my $ref = format_ref_marker($refs, $hash_base);
git_header_html();
my $basedir = '';
- my ($have_blame) = gitweb_check_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
if (defined $hash_base && (my %co = parse_commit($hash_base))) {
my @views_nav = ();
if (defined $file_name) {
@@ -5856,7 +5856,7 @@ insensitive).</p>
<dt><b>commit</b></dt>
<dd>The commit messages and authorship information will be scanned for the given pattern.</dd>
EOT
- my ($have_grep) = gitweb_check_feature('grep');
+ my $have_grep = gitweb_check_feature('grep');
if ($have_grep) {
print <<EOT;
<dt><b>grep</b></dt>
@@ -5873,7 +5873,7 @@ EOT
<dt><b>committer</b></dt>
<dd>Name and e-mail of the committer and date of commit will be scanned for the given pattern.</dd>
EOT
- my ($have_pickaxe) = gitweb_check_feature('pickaxe');
+ my $have_pickaxe = gitweb_check_feature('pickaxe');
if ($have_pickaxe) {
print <<EOT;
<dt><b>pickaxe</b></dt>
@@ -5925,7 +5925,7 @@ sub git_shortlog {
sub git_feed {
my $format = shift || 'atom';
- my ($have_blame) = gitweb_check_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
# Atom: http://www.atomenabled.org/developers/syndication/
# RSS: http://www.notestips.com/80256B3A007F2692/1/NAMO5P9UPQ
--
1.5.6.5
^ permalink raw reply related
* Re: [PATCHv2 2/2] gitweb: clean up gitweb_check_feature() calls
From: Jakub Narebski @ 2008-11-29 11:18 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Junio C Hamano
In-Reply-To: <1227957301-7417-1-git-send-email-giuseppe.bilotta@gmail.com>
Giuseppe Bilotta wrote:
> Since git_check_feature now returns a scalar, the parentheses around the
^^^
\---- missed one.
> variables initialized from gitweb_check_feature() are not needed.
>
> We remove them for stylistic consistency and to prevent unnecessary
> ambiguity in the code.
>
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> ---
>
> Fixed the commit message, use this instead of the previous one.
--
Jakub Narebski
Poland
^ permalink raw reply
* [PATCHv2ter 2/2] gitweb: clean up gitweb_check_feature() calls
From: Giuseppe Bilotta @ 2008-11-29 11:53 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Giuseppe Bilotta
In-Reply-To: <1227904793-1821-3-git-send-email-giuseppe.bilotta@gmail.com>
Since gitweb_check_feature now returns a scalar, the parentheses around
the variables initialized from gitweb_check_feature() are not needed.
We remove them for stylistic consistency and to prevent unnecessary
ambiguity in the code.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
Third attempt, hopefully I got the commit message right, finally 8-P
gitweb/gitweb.perl | 26 +++++++++++++-------------
1 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 128d7ad..b0d00ea 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -828,7 +828,7 @@ sub href (%) {
}
}
- my ($use_pathinfo) = gitweb_check_feature('pathinfo');
+ my $use_pathinfo = gitweb_check_feature('pathinfo');
if ($use_pathinfo) {
# try to put as many parameters as possible in PATH_INFO:
# - project name
@@ -2119,7 +2119,7 @@ sub git_get_projects_list {
$filter ||= '';
$filter =~ s/\.git$//;
- my ($check_forks) = gitweb_check_feature('forks');
+ my $check_forks = gitweb_check_feature('forks');
if (-d $projects_list) {
# search in directory
@@ -2965,7 +2965,7 @@ EOF
}
print "</div>\n";
- my ($have_search) = gitweb_check_feature('search');
+ my $have_search = gitweb_check_feature('search');
if (defined $project && $have_search) {
if (!defined $searchtext) {
$searchtext = "";
@@ -2979,7 +2979,7 @@ EOF
$search_hash = "HEAD";
}
my $action = $my_uri;
- my ($use_pathinfo) = gitweb_check_feature('pathinfo');
+ my $use_pathinfo = gitweb_check_feature('pathinfo');
if ($use_pathinfo) {
$action .= "/".esc_url($project);
}
@@ -3472,7 +3472,7 @@ sub is_patch_split {
sub git_difftree_body {
my ($difftree, $hash, @parents) = @_;
my ($parent) = $parents[0];
- my ($have_blame) = gitweb_check_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
print "<div class=\"list_head\">\n";
if ($#{$difftree} > 10) {
print(($#{$difftree} + 1) . " files changed:\n");
@@ -3986,7 +3986,7 @@ sub git_project_list_body {
# actually uses global variable $project
my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
- my ($check_forks) = gitweb_check_feature('forks');
+ my $check_forks = gitweb_check_feature('forks');
my @projects = fill_project_list_info($projlist, $check_forks);
$order ||= $default_projects_order;
@@ -4446,7 +4446,7 @@ sub git_summary {
my @taglist = git_get_tags_list(16);
my @headlist = git_get_heads_list(16);
my @forklist;
- my ($check_forks) = gitweb_check_feature('forks');
+ my $check_forks = gitweb_check_feature('forks');
if ($check_forks) {
@forklist = git_get_projects_list($project);
@@ -4475,7 +4475,7 @@ sub git_summary {
}
# Tag cloud
- my $show_ctags = (gitweb_check_feature('ctags'))[0];
+ my $show_ctags = gitweb_check_feature('ctags');
if ($show_ctags) {
my $ctags = git_get_project_ctags($project);
my $cloud = git_populate_project_tagcloud($ctags);
@@ -4765,7 +4765,7 @@ sub git_blob {
$expires = "+1d";
}
- my ($have_blame) = gitweb_check_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
open my $fd, "-|", git_cmd(), "cat-file", "blob", $hash
or die_error(500, "Couldn't cat $file_name, $hash");
my $mimetype = blob_mimetype($fd, $file_name);
@@ -4858,7 +4858,7 @@ sub git_tree {
my $ref = format_ref_marker($refs, $hash_base);
git_header_html();
my $basedir = '';
- my ($have_blame) = gitweb_check_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
if (defined $hash_base && (my %co = parse_commit($hash_base))) {
my @views_nav = ();
if (defined $file_name) {
@@ -5856,7 +5856,7 @@ insensitive).</p>
<dt><b>commit</b></dt>
<dd>The commit messages and authorship information will be scanned for the given pattern.</dd>
EOT
- my ($have_grep) = gitweb_check_feature('grep');
+ my $have_grep = gitweb_check_feature('grep');
if ($have_grep) {
print <<EOT;
<dt><b>grep</b></dt>
@@ -5873,7 +5873,7 @@ EOT
<dt><b>committer</b></dt>
<dd>Name and e-mail of the committer and date of commit will be scanned for the given pattern.</dd>
EOT
- my ($have_pickaxe) = gitweb_check_feature('pickaxe');
+ my $have_pickaxe = gitweb_check_feature('pickaxe');
if ($have_pickaxe) {
print <<EOT;
<dt><b>pickaxe</b></dt>
@@ -5925,7 +5925,7 @@ sub git_shortlog {
sub git_feed {
my $format = shift || 'atom';
- my ($have_blame) = gitweb_check_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
# Atom: http://www.atomenabled.org/developers/syndication/
# RSS: http://www.notestips.com/80256B3A007F2692/1/NAMO5P9UPQ
--
1.5.6.5
^ permalink raw reply related
* Re: [JGIT PATCH 0/4] RepositoryTestCase cleanups
From: Robin Rosenberg @ 2008-11-29 12:01 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, fonseca
In-Reply-To: <20081127214916.GD23984@spearce.org>
torsdag 27 november 2008 22:49:16 skrev Shawn O. Pearce:
> Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > Ok, so here is an attempt to improve the ability of the JGit's unit
> > tests to delete temporary repositories. This has probably been seen
> > by many, but Jonas Fonseca raised the issue.
>
> Hmpph. This takes 19 seconds to run the suite, where it used to be
> only 2 seconds on the same system. The slower run isn't something
> I'm too happy about, actually I'd like to make the run even faster
> than 2 seconds.
So maybe we should clean up less. Every new test repo we create has
a new name so we could do without cleaning up so much. The cleanup
however is a verification that we close (and can close) our resources,
though it only works on Windows :/ On unix we could spawn lsof but that
is really really slow.
> If the issue is mmap'd files, why don't we instead disable mmap
> on Windows during JUnit tests, and use the non-mmap variant of
> pack access? At least do that for the bulk of the tests, and
> then have a single test case which tests the mmap code path but
> has careful System.gc calls in place to try and ensure we can
> actually clean up the temporary files.
We would then need some other really slow test to play rough with
memory mapping and gc. As I mentioned above it is actually about
closing resources in general, mmapped files being an especially
nasty case.
> I'd actually like to get that Repository refactoring done soon,
> someone else was asking about it for the RefDatabase (to store the
> refs in a SQL database so JGit ties into JTA) but I may also want
> it for Gerrit 2 - I'm looking at doing something that would put
> 200,000 refs per year into a repository. That's so large that
> most operations can't afford to scan the entire ref database,
> and it really cannot be loose. ;-)
Would be cool, but having that diff engine is more important to me.
>
>
> Refactoring repository is a fair chunk of work, disabling the mmap
> feature under Windows in JUnit may be easier. Hmm, according to
> WindowCache's <clinit> its default by false. Why is it enabling
> on Windows? The only code that calls WindowCache.reconfigure()
> is in the Eclipse plugin, so pure JGit unit tests shouldn't be
> turning on mmap code *at all*.
>
> Which also points out a gap in our tests. Nothing new, we have
> lots of gaps. *sigh*
Yep. :/
-- robin
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)
From: Nguyen Thai Ngoc Duy @ 2008-11-29 13:02 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, git
In-Reply-To: <alpine.LNX.1.00.0811281938250.19665@iabervon.org>
On 11/29/08, Daniel Barkalow <barkalow@iabervon.org> wrote:
> On Fri, 28 Nov 2008, Junio C Hamano wrote:
>
> > "Shawn O. Pearce" <spearce@spearce.org> writes:
> >
> > > Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > ...
> > >> In other words, unless there is more interest in that feature, enough to
> > >> generate a well-understood design before a good implementation, I'd rather
> > >> see this patch series dropped.
> > >
> > > Ack. I agree with every remark made by Dscho, and also want to cry "wolf".
> > >
> > > I haven't had time to read the patch series. Its big and intrusive
> > > and I just don't need the feature.
> >
> > Well, "me neither". Although I personally think resisting changes until
> > it becomes absolutely necessary is a good discipline, we also need to
> > recognise that there is a chicken-and-egg problem. When you have a
> > potentially useful feature, unless people actually try using it in the
> > field, you won't discover the drawbacks in either the design nor the
> > implementation, let alone any improvements.
>
>
> I just looked over most of it (skipping the generic index extension
> portion). It looks to me like it's introducing an extra concept to avoid
> actually fixing maybe-bugs in the "assume unchanged" implementation
> when used with files that have been changed intentionally (with the user
> intending git to overlook this change). Sparse checkout is essentially a
> special case of this, where the user has changed the working directory
> radically (not populating it at all) and wants git to carry on as if this
> was not the case (with a certain amount of porcelain code to cause this to
> happen automatically).
I chose to use another bit because I did not want to change the
behaviour of CE_VALID bit: it assumes all files are present.
> If there's any need for this to be distinguished from "assume unchanged",
> I think it should be used with, not instead of, the CE_VALID bit; and it
> could probably use some bit in the stat info section, since we don't need
> stat info if we know by assumption that the entry is valid.
Interesting. I'll think more about this.
> -Daniel
> *This .sig left intentionally blank*
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Duy
^ permalink raw reply
* [PATCH 0/2] gitweb: patch view
From: Giuseppe Bilotta @ 2008-11-29 13:41 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Giuseppe Bilotta
I recently discovered that the commitdiff_plain view is not exactly
something that can be used by git am directly (for example, the subject
line gets duplicated in the commit message body after using git am).
Since I'm not sure if it was the case to fix the plain view because I
don't know what its intended usage was, I prepared a new view,
uncreatively called 'patch', that exposes git format-patch output
directly.
The second patch exposes it from commitdiff view (obviosly), but also
from shortlog view, when less than 16 patches are begin shown.
Giuseppe Bilotta (2):
gitweb: add patch view
gitweb: links to patch action in commitdiff and shortlog view
gitweb/gitweb.perl | 35 +++++++++++++++++++++++++++++++++--
1 files changed, 33 insertions(+), 2 deletions(-)
^ permalink raw reply
* [PATCH 2/2] gitweb: links to patch action in commitdiff and shortlog view
From: Giuseppe Bilotta @ 2008-11-29 13:41 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Giuseppe Bilotta
In-Reply-To: <1227966071-11104-2-git-send-email-giuseppe.bilotta@gmail.com>
The link from commitdiff view is an obviously needed one, but we also
offer the option to link to the patchset in shortlog view, when there
are less than 15 commits being shown.
---
gitweb/gitweb.perl | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index befe6b6..5b18fdf 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5382,7 +5382,9 @@ sub git_commitdiff {
if ($format eq 'html') {
$formats_nav =
$cgi->a({-href => href(action=>"commitdiff_plain", -replay=>1)},
- "raw");
+ "raw") . " | " .
+ $cgi->a({-href => href(action=>"patch", -replay=>1)},
+ "patch");
if (defined $hash_parent &&
$hash_parent ne '-c' && $hash_parent ne '--cc') {
@@ -5915,6 +5917,12 @@ sub git_shortlog {
$cgi->a({-href => href(-replay=>1, page=>$page+1),
-accesskey => "n", -title => "Alt-n"}, "next");
}
+ # TODO this should be configurable
+ if ($#commitlist <= 15) {
+ $paging_nav .= " ⋅ " .
+ $cgi->a({-href => href(action=>"patch", -replay=>1)},
+ $#commitlist > 1 ? "patchset" : "patch");
+ }
git_header_html();
git_print_page_nav('shortlog','', $hash,$hash,$hash, $paging_nav);
--
1.5.6.5
^ permalink raw reply related
* [PATCH 1/2] gitweb: add patch view
From: Giuseppe Bilotta @ 2008-11-29 13:41 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Giuseppe Bilotta
In-Reply-To: <1227966071-11104-1-git-send-email-giuseppe.bilotta@gmail.com>
Trying to use 'commitdiff_plain' output as input to git am results in
some annoying results such as doubled subject lines. We thus offer a new
'patch' view that exposes format-patch output directly. This makes it
easier to offer patches by linking to gitweb repositories.
---
gitweb/gitweb.perl | 25 ++++++++++++++++++++++++-
1 files changed, 24 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 933e137..befe6b6 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -485,6 +485,7 @@ our %actions = (
"heads" => \&git_heads,
"history" => \&git_history,
"log" => \&git_log,
+ "patch" => \&git_patch,
"rss" => \&git_rss,
"atom" => \&git_atom,
"search" => \&git_search,
@@ -5465,7 +5466,11 @@ sub git_commitdiff {
open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
'-p', $hash_parent_param, $hash, "--"
or die_error(500, "Open git-diff-tree failed");
-
+ } elsif ($format eq 'patch') {
+ open $fd, "-|", git_cmd(), "format-patch", '--stdout',
+ $hash_parent ? "$hash_parent..$hash" :
+ ('--root', '-1', $hash)
+ or die_error(500, "Open git-format-patch failed");
} else {
die_error(400, "Unknown commitdiff format");
}
@@ -5514,6 +5519,14 @@ sub git_commitdiff {
print to_utf8($line) . "\n";
}
print "---\n\n";
+ } elsif ($format eq 'patch') {
+ my $filename = basename($project) . "-$hash.patch";
+
+ print $cgi->header(
+ -type => 'text/plain',
+ -charset => 'utf-8',
+ -expires => $expires,
+ -content_disposition => 'inline; filename="' . "$filename" . '"');
}
# write patch
@@ -5535,6 +5548,11 @@ sub git_commitdiff {
print <$fd>;
close $fd
or print "Reading git-diff-tree failed\n";
+ } elsif ($format eq 'patch') {
+ local $/ = undef;
+ print <$fd>;
+ close $fd
+ or print "Reading git-format-patch failed\n";
}
}
@@ -5542,6 +5560,11 @@ sub git_commitdiff_plain {
git_commitdiff('plain');
}
+# format-patch-style patches
+sub git_patch {
+ git_commitdiff('patch');
+}
+
sub git_history {
if (!defined $hash_base) {
$hash_base = git_get_head_hash($project);
--
1.5.6.5
^ permalink raw reply related
* Re: C# Git Implementation
From: Andreas Ericsson @ 2008-11-29 13:44 UTC (permalink / raw)
To: JD Guzman; +Cc: git
In-Reply-To: <001501c9515e$66e8ac70$34ba0550$@com>
JD Guzman wrote:
> Hello I am new to the list and if I am not following some kind of list
> ettiquite please let me know.
>
> I read in the archives that there was once talk of porting Git over to C#
> and was wondering if anything ever came of this?
That depends. I think it's happened at least on three different occasions
that someone wants to port git to C#. The latest is the attempt to write a
libified git (in C) that will be DllImport()'ed from C#, which the devs at
MonoDevelop were sort of interested in. I'm currently working (very slowly)
on libgit2, and it's nowhere near usable today.
> I realize there were some
> that didn't see the use for this but as Git is becoming more and more
> popular a more native implementation of Git for windows users would be a
> good endevour IMHO.
>
Truly. However, a better way forward is to create a portable git library
that any old application can use, no matter what language the application
is written in. That way, GUI-hackers can add all the bells and whistles
they like.
Besides that, msysgit works very well on Windows.
> At any rate any info would be greatly appreciated.
>
libgit2 is available from
http://www.spearce.org/projects/scm/libgit2/libgit2.git
git://git.op5.org/git/libgit2.git
The first URL belongs to Shawn O. Pearce, the initiator of the libgit2
project. The second one is where I stash random stuff that I work on.
Note that git.op5.org is subject to heavy rebasing at the moment, as I'm
basically using it as a patch queue for Shawn (and everyone else who's
interested) to look at when they feel like it. It's not yet interesting
enough to warrant sending everything as patches to git@vger, but a small
bunch of people are helping making the small internal API's reliably
portable (filesystem operations, thread-local error handling) so that
we can nail them down hard to prevent having to change them (and their
hundreds of callers) in the future.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH 2/3] git add --intent-to-add: fix removal of cached emptiness
From: Sverre Rabbelier @ 2008-11-29 15:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, Johannes Schindelin, git
In-Reply-To: <7vprkf2nki.fsf_-_@gitster.siamese.dyndns.org>
On Sat, Nov 29, 2008 at 04:55, Junio C Hamano <gitster@pobox.com> wrote:
> This uses the extended index flag mechanism introduced earlier to mark
> the entries added to the index via "git add -N" with CE_INTENT_TO_ADD.
Is 'intent' [0] used properly here? Should it not be 'intend' [1]?
[0] http://en.wiktionary.org/wiki/intent
[1] http://en.wiktionary.org/wiki/intend
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH 1/2] gitweb: add patch view
From: Sverre Rabbelier @ 2008-11-29 15:43 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Jakub Narebski, Petr Baudis, Junio C Hamano
In-Reply-To: <1227966071-11104-2-git-send-email-giuseppe.bilotta@gmail.com>
On Sat, Nov 29, 2008 at 14:41, Giuseppe Bilotta
<giuseppe.bilotta@gmail.com> wrote:
> Trying to use 'commitdiff_plain' output as input to git am results in
> some annoying results such as doubled subject lines. We thus offer a new
> 'patch' view that exposes format-patch output directly. This makes it
> easier to offer patches by linking to gitweb repositories.
If this does what I think it does I would be very happy with this
feature :). Only yesterday I wanted to link someone to a patch I put
up on repo.or.cz, but instead ended up telling them to download the
snapshot.
As an additional feature request; would it be possible to have a view
that exposes a patch that is directly applyable by the patch command?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH 1/2] gitweb: add patch view
From: Jakub Narebski @ 2008-11-29 16:10 UTC (permalink / raw)
To: sverre; +Cc: Giuseppe Bilotta, git, Petr Baudis, Junio C Hamano
In-Reply-To: <bd6139dc0811290743s6cf8e534nddd8a09698ea22b9@mail.gmail.com>
On Sat, 29 Nov 2008, Sverre Rabbelier wrote:
> On Sat, Nov 29, 2008 at 14:41, Giuseppe Bilotta
> <giuseppe.bilotta@gmail.com> wrote:
> > Trying to use 'commitdiff_plain' output as input to git am results in
> > some annoying results such as doubled subject lines. We thus offer a new
> > 'patch' view that exposes format-patch output directly. This makes it
> > easier to offer patches by linking to gitweb repositories.
>
> If this does what I think it does I would be very happy with this
> feature :). Only yesterday I wanted to link someone to a patch I put
> up on repo.or.cz, but instead ended up telling them to download the
> snapshot.
True. 'commitdiff_plain' wasn't good enough; what's more it suffers
from the same ambiguity as 'commitdiff', i.e. it is both means to
show diff _for_ a commit (perhaps selecting one of parents), and
showing diff _between_ two commits; the new 'patch' always shows
diff for a commit, or for a series of commits.
>
> As an additional feature request; would it be possible to have a view
> that exposes a patch that is directly applyable by the patch command?
Both new 'patch' ('patchset') and old 'blobdiff_plain' should be
directly applicable by patch and by git-apply.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH 1/2] gitweb: add patch view
From: Giuseppe Bilotta @ 2008-11-29 16:48 UTC (permalink / raw)
To: Jakub Narebski; +Cc: sverre, git, Petr Baudis, Junio C Hamano
In-Reply-To: <200811291710.27891.jnareb@gmail.com>
Wow, I honestly didn't expect this idea to be so successful. I thought
I was the only one using gitweb to send patches around, honestly 8-D
On Sat, Nov 29, 2008 at 5:10 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Sat, 29 Nov 2008, Sverre Rabbelier wrote:
>>
>> If this does what I think it does I would be very happy with this
>> feature :). Only yesterday I wanted to link someone to a patch I put
>> up on repo.or.cz, but instead ended up telling them to download the
>> snapshot.
>
> True. 'commitdiff_plain' wasn't good enough; what's more it suffers
> from the same ambiguity as 'commitdiff', i.e. it is both means to
> show diff _for_ a commit (perhaps selecting one of parents), and
> showing diff _between_ two commits; the new 'patch' always shows
> diff for a commit, or for a series of commits.
Maybe commitdiff should me renamed to just be 'diff'.
Also, I was in doubt about the name for the new view, and I did
consider 'patchset' (which you mention in your email). I chose to
stick with the shorter form in the end, since many people complain
that gitweb already produces paths that are too long.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* more merge strategies : feature request
From: Caleb Cushing @ 2008-11-29 16:48 UTC (permalink / raw)
To: git
conflict: this strategy would always resolve in a merge conflict
allowing you to use git mergetool to piece the files back together.
no-overwrite: if a change from the branch being merged in would
overwrite something in the current branch don't merge it. (I think it
needs a better name)
--
Caleb Cushing
^ permalink raw reply
* Re: [PATCH 1/2] gitweb: add patch view
From: Sverre Rabbelier @ 2008-11-29 16:50 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: Jakub Narebski, git, Petr Baudis, Junio C Hamano
In-Reply-To: <cb7bb73a0811290848j1b77fe89m66ead7cc4f5ca2bb@mail.gmail.com>
On Sat, Nov 29, 2008 at 17:48, Giuseppe Bilotta
<giuseppe.bilotta@gmail.com> wrote:
> Also, I was in doubt about the name for the new view, and I did
> consider 'patchset' (which you mention in your email). I chose to
> stick with the shorter form in the end, since many people complain
> that gitweb already produces paths that are too long.
Heh, what do I care about the length of the url, there's so many url
shorteners out there, that's not really a problem to me :).
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: C# Git Implementation
From: Asheesh Laroia @ 2008-11-29 18:08 UTC (permalink / raw)
To: JD Guzman; +Cc: git
In-Reply-To: <001501c9515e$66e8ac70$34ba0550$@com>
On Fri, 28 Nov 2008, JD Guzman wrote:
> I read in the archives that there was once talk of porting Git over to
> C# and was wondering if anything ever came of this? I realize there
> were some that didn't see the use for this but as Git is becoming more
> and more popular a more native implementation of Git for windows users
> would be a good endevour IMHO.
Is it possible to compile the Java-language jgit with the Common Language
Runtime and access that from a C# application?
-- Asheesh.
--
Your talents will be recognized and suitably rewarded.
^ permalink raw reply
* A better approach to diffing and merging
From: Ian Clarke @ 2008-11-29 18:12 UTC (permalink / raw)
To: git
Apologies if this is off-topic, but I recently had an idea for a
better way to do diffs and merging which I thought may be of interest
to those on this list.
I described it in a blog entry here: http://budurl.com/jyt6
For your convenience text is pasted below (although missing a few hyperlinks):
A plan for better source code diffs and merging
======================================
I've been using Subversion for years, but a few months ago I was
really starting to feel the limitations of being able to create and
merge branches easily. I'd heard that Git made this very easy indeed,
and so I decided to try it.
Anyway, this isn't yet another "I discovered Git and now I've achieved
self-actualization" blog post, so to cut a long story short, I now use
git for everything (together with the excellent GitHub).
Even though merging is a lot better with Git than Subversion, I've
still found myself getting into situations where it requires a lot of
work to merge a branch back into another branch, and it got me
thinking about better ways to do merging.
While I'm no merging expert, it seems that most merging algorithms do
it on a line-by-line basis, treating source code as nothing but a list
of lines of text. It got me thinking, what if the merging algorithm
understood the structure of the source code it is trying to merge?
So the idea is this:
Provide the merge algorithm with the grammar of the programming
language, perhaps in the form of a Bison grammar file, or some other
standardized way to represent a grammar.
The merge algorithm then uses this to parse the files to be diffed
and/or merged into trees, and then the diff and merge are treated as
operations on these trees. These operations may include creating,
deleting, or moving nodes or branches, renaming nodes, etc. There has
been quite a bit (pdf) of academic research on this topic, although I
haven't yet found off-the-shelf code that will do what we need.
Still, it shouldn't be terribly hard to implement.
The beauty of this approach is that the merge algorithm should be far
less likely to be confused by formatting changes, and much more likely
to correctly identify what has changed.
I can't think of any reason that such a tool wouldn't work in the
exact same way as existing diff/merge tools from the programmer's
perspective. The tool would automatically select the correct grammar
based on the file extension, or fall-back to line-based diffs if the
extension is unrecognized (or the file isn't valid for the selected
grammar). Thus, it should be trivial to use this new tool with
existing version control systems.
I'd love to have the time to implement this, although regretfully it
is at the bottom of a very large pile of "some day" projects. I think
this is an interesting enough idea, and one that would be immediately
useful, that if I put it out there someone somewhere might be able to
make it a reality.
Any takers? I've set up a Google Group for further discussion, please
join if interested.
^ permalink raw reply
* Overwriting untracked working tree files (when identical)
From: Asheesh Laroia @ 2008-11-29 18:58 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: TEXT/PLAIN, Size: 576 bytes --]
I was using git earlier today and got the familiar error:
"Untracked working tree file 'filename’ would be overwritten by merge"
As it happens, I know that filename in my local filesystem and filename in
the remote git branch have the same contents.
Would it make sense for git to check for this possibility, and if the
working tree file is the same as the file being merged in with that
filename, allow the overwrite?
(I received the message with "git pull," so I imagine it was emitted by
the recursive merge driver.)
-- Asheesh.
--
Reply hazy, ask again later.
^ permalink raw reply
* Re: C# Git Implementation
From: Alejandro Riveira @ 2008-11-29 19:27 UTC (permalink / raw)
To: git
In-Reply-To: <alpine.DEB.2.00.0811291308120.17643@vellum.laroia.net>
El Sat, 29 Nov 2008 13:08:58 -0500, Asheesh Laroia escribió:
> On Fri, 28 Nov 2008, JD Guzman wrote:
>
>> I read in the archives that there was once talk of porting Git over to
>> C# and was wondering if anything ever came of this? I realize there
>> were some that didn't see the use for this but as Git is becoming more
>> and more popular a more native implementation of Git for windows users
>> would be a good endevour IMHO.
>
> Is it possible to compile the Java-language jgit with the Common
> Language Runtime and access that from a C# application?
ikvm ?? http://www.ikvm.net/
>
> -- Asheesh.
^ permalink raw reply
* Re: C# Git Implementation
From: Robin Rosenberg @ 2008-11-29 19:55 UTC (permalink / raw)
To: Asheesh Laroia; +Cc: JD Guzman, git
In-Reply-To: <alpine.DEB.2.00.0811291308120.17643@vellum.laroia.net>
lördag 29 november 2008 19:08:58 skrev Asheesh Laroia:
> On Fri, 28 Nov 2008, JD Guzman wrote:
>
> > I read in the archives that there was once talk of porting Git over to
> > C# and was wondering if anything ever came of this? I realize there
> > were some that didn't see the use for this but as Git is becoming more
> > and more popular a more native implementation of Git for windows users
> > would be a good endevour IMHO.
>
> Is it possible to compile the Java-language jgit with the Common Language
> Runtime and access that from a C# application?
>
> -- Asheesh.
>
I tried it half a year ago. Recompiling and running was easy, Unfortunately it hung on reading
blobs and the stacktrace showed it was stuck in deflate. It's not very high on my list of priorities,
but I assume fixing the problem is probably not hard if you decide to get your editor dirty.
-- robin
^ permalink raw reply
* Re: [PATCHv2ter 2/2] gitweb: clean up gitweb_check_feature() calls
From: Junio C Hamano @ 2008-11-29 20:39 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Jakub Narebski, Petr Baudis
In-Reply-To: <1227959616-8056-1-git-send-email-giuseppe.bilotta@gmail.com>
Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
> Since gitweb_check_feature now returns a scalar, the parentheses around
> the variables initialized from gitweb_check_feature() are not needed.
Here is what I think the commit message of your [1/2] should have read:
Subject: [PATCH] gitweb: make gitweb_check_feature a boolean wrapper
The gitweb_check_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 a 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 this is a bug waiting to
happen for future callers that are not careful.
This changes gitweb_check_feature() to a wrapper that can be called
safely in the scalar context to see if a feature is enabled to reduce
the risk of future bugs. A new function gitweb_get_feature() is
introduced for the callers that want the list of configuration
parameters. All existing callers of gitweb_check_feature() that want
the configuration parameters are adjusted to call gitweb_get_feature().
Notice the difference in the tone from your [1/2]. The change is not
about fixing (your proposed commit log message read "gitweb:fixes to
gitweb feature check code") as nothing is broken. It is purely about
futureproofing and I think we should mark it as such.
The splitting of two patches is not strictly necessary, but if you wanted
to, I would do so slightly differently:
* Make [1/2] purely the rename of s/check/get/, saying "the function is
not just checking if it is enabled, but it is getting the configuration
parameter list. I'll introduce a new API to ask if it is enabled,
which will be called 'check', hence this rename".
Obviously, after applying this patch, there should be no caller of
"check".
* Make [2/2] introduce the "check" wrapper, and change the existing
callers of "get" that want boolean to call "check" (and in the scalar
context, as it does not matter anymore). You can steal most of the
above rewrite as your commit message for this (except for the last two
sentences).
Also I would rewrite the mysterious "Comfort code" comment like this:
# A wrapper to check if a given feature is enabled.
# With this, you can say
#
# my $bool_feat = gitweb_check_feature('bool_feat');
# gitweb_check_feature('bool_feat') or somecode;
#
# instead of
#
# my ($bool_feat) = gitweb_get_feature('bool_feat');
# (gitweb_get_feature('bool_feat'))[0] or somecode;
(you also had a few typos in these four example calls).
^ permalink raw reply
* Re: [PATCHv2ter 2/2] gitweb: clean up gitweb_check_feature() calls
From: Junio C Hamano @ 2008-11-29 21:00 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Jakub Narebski, Petr Baudis
In-Reply-To: <7vej0u1d2c.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> ... The change is not
> about fixing (your proposed commit log message read "gitweb:fixes to
> gitweb feature check code") as nothing is broken. It is purely about
> futureproofing and I think we should mark it as such.
Actually, there is a handful clueless/careless callers. Before applying
any of the check => get patch, let's do this as a fix.
-- >8 --
Subject: [PATCH] gitweb: fix 'ctags' feature check and others
gitweb_check_feature() function is to retrieve the configuration parameter
list and calling it in a 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 list context.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
gitweb/gitweb.perl | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 933e137..400f5c8 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3914,7 +3914,7 @@ sub fill_project_list_info {
my ($projlist, $check_forks) = @_;
my @projects;
- my $show_ctags = gitweb_check_feature('ctags');
+ my ($show_ctags) = gitweb_check_feature('ctags');
PROJECT:
foreach my $pr (@$projlist) {
my (@activity) = git_get_last_activity($pr->{'path'});
@@ -3988,7 +3988,7 @@ sub git_project_list_body {
@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;
}
- my $show_ctags = gitweb_check_feature('ctags');
+ my ($show_ctags) = gitweb_check_feature('ctags');
if ($show_ctags) {
my %ctags;
foreach my $p (@projects) {
@@ -4457,7 +4457,7 @@ sub git_summary {
}
# Tag cloud
- my $show_ctags = (gitweb_check_feature('ctags'))[0];
+ my ($show_ctags) = gitweb_check_feature('ctags');
if ($show_ctags) {
my $ctags = git_get_project_ctags($project);
my $cloud = git_populate_project_tagcloud($ctags);
@@ -4559,7 +4559,7 @@ sub git_blame {
my $fd;
my $ftype;
- gitweb_check_feature('blame')
+ gitweb_check_feature('blame')[0]
or die_error(403, "Blame view not allowed");
die_error(400, "No file name given") unless $file_name;
@@ -5610,7 +5610,7 @@ sub git_history {
}
sub git_search {
- gitweb_check_feature('search') or die_error(403, "Search is disabled");
+ gitweb_check_feature('search')[0] or die_error(403, "Search is disabled");
if (!defined $searchtext) {
die_error(400, "Text field is empty");
}
@@ -5629,11 +5629,11 @@ sub git_search {
if ($searchtype eq 'pickaxe') {
# pickaxe may take all resources of your box and run for several minutes
# with every query - so decide by yourself how public you make this feature
- gitweb_check_feature('pickaxe')
+ gitweb_check_feature('pickaxe')[0]
or die_error(403, "Pickaxe is disabled");
}
if ($searchtype eq 'grep') {
- gitweb_check_feature('grep')
+ gitweb_check_feature('grep')[0]
or die_error(403, "Grep is disabled");
}
--
1.6.0.4.850.g6bd829
^ permalink raw reply related
* [PATCH 2/3] gitweb: rename gitweb_check_feature to gitweb_get_feature
From: Junio C Hamano @ 2008-11-29 21:12 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Jakub Narebski, Petr Baudis
In-Reply-To: <7vzljiz1qn.fsf@gitster.siamese.dyndns.org>
The function is about retrieving the configuration parameter list for the
feature. A more robust way to check if a feature is enabled is introduced
in the next patch, and the function will be called gitweb_check_feature.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* The earlier "fix ctags and friends" is the 1/3; this shows how I would
have liked to see your patches as a reviewable series. The end result
is almost identical.
gitweb/gitweb.perl | 46 +++++++++++++++++++++++-----------------------
1 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 400f5c8..7ab16cd 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -190,7 +190,7 @@ our %feature = (
# if there is no 'sub' key (no feature-sub), then feature cannot be
# overriden
#
- # use gitweb_check_feature(<feature>) to check if <feature> is enabled
+ # use gitweb_get_feature(<feature>) to check if <feature> is enabled
# Enable the 'blame' blob view, showing the last commit that modified
# each line in the file. This can be very CPU-intensive.
@@ -329,7 +329,7 @@ our %feature = (
'default' => [0]},
);
-sub gitweb_check_feature {
+sub gitweb_get_feature {
my ($name) = @_;
return unless exists $feature{$name};
my ($sub, $override, @defaults) = (
@@ -767,7 +767,7 @@ our $git_dir;
$git_dir = "$projectroot/$project" if $project;
# list of supported snapshot formats
-our @snapshot_fmts = gitweb_check_feature('snapshot');
+our @snapshot_fmts = gitweb_get_feature('snapshot');
@snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);
# dispatch
@@ -810,7 +810,7 @@ sub href (%) {
}
}
- my ($use_pathinfo) = gitweb_check_feature('pathinfo');
+ my ($use_pathinfo) = gitweb_get_feature('pathinfo');
if ($use_pathinfo) {
# try to put as many parameters as possible in PATH_INFO:
# - project name
@@ -2101,7 +2101,7 @@ sub git_get_projects_list {
$filter ||= '';
$filter =~ s/\.git$//;
- my ($check_forks) = gitweb_check_feature('forks');
+ my ($check_forks) = gitweb_get_feature('forks');
if (-d $projects_list) {
# search in directory
@@ -2947,7 +2947,7 @@ EOF
}
print "</div>\n";
- my ($have_search) = gitweb_check_feature('search');
+ my ($have_search) = gitweb_get_feature('search');
if (defined $project && $have_search) {
if (!defined $searchtext) {
$searchtext = "";
@@ -2961,7 +2961,7 @@ EOF
$search_hash = "HEAD";
}
my $action = $my_uri;
- my ($use_pathinfo) = gitweb_check_feature('pathinfo');
+ my ($use_pathinfo) = gitweb_get_feature('pathinfo');
if ($use_pathinfo) {
$action .= "/".esc_url($project);
}
@@ -3084,7 +3084,7 @@ sub git_print_page_nav {
$arg{'tree'}{'hash'} = $treehead if defined $treehead;
$arg{'tree'}{'hash_base'} = $treebase if defined $treebase;
- my @actions = gitweb_check_feature('actions');
+ my @actions = gitweb_get_feature('actions');
my %repl = (
'%' => '%',
'n' => $project, # project name
@@ -3454,7 +3454,7 @@ sub is_patch_split {
sub git_difftree_body {
my ($difftree, $hash, @parents) = @_;
my ($parent) = $parents[0];
- my ($have_blame) = gitweb_check_feature('blame');
+ my ($have_blame) = gitweb_get_feature('blame');
print "<div class=\"list_head\">\n";
if ($#{$difftree} > 10) {
print(($#{$difftree} + 1) . " files changed:\n");
@@ -3914,7 +3914,7 @@ sub fill_project_list_info {
my ($projlist, $check_forks) = @_;
my @projects;
- my ($show_ctags) = gitweb_check_feature('ctags');
+ my ($show_ctags) = gitweb_get_feature('ctags');
PROJECT:
foreach my $pr (@$projlist) {
my (@activity) = git_get_last_activity($pr->{'path'});
@@ -3968,7 +3968,7 @@ sub git_project_list_body {
# actually uses global variable $project
my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
- my ($check_forks) = gitweb_check_feature('forks');
+ my ($check_forks) = gitweb_get_feature('forks');
my @projects = fill_project_list_info($projlist, $check_forks);
$order ||= $default_projects_order;
@@ -3988,7 +3988,7 @@ sub git_project_list_body {
@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;
}
- my ($show_ctags) = gitweb_check_feature('ctags');
+ my ($show_ctags) = gitweb_get_feature('ctags');
if ($show_ctags) {
my %ctags;
foreach my $p (@projects) {
@@ -4428,7 +4428,7 @@ sub git_summary {
my @taglist = git_get_tags_list(16);
my @headlist = git_get_heads_list(16);
my @forklist;
- my ($check_forks) = gitweb_check_feature('forks');
+ my ($check_forks) = gitweb_get_feature('forks');
if ($check_forks) {
@forklist = git_get_projects_list($project);
@@ -4457,7 +4457,7 @@ sub git_summary {
}
# Tag cloud
- my ($show_ctags) = gitweb_check_feature('ctags');
+ my ($show_ctags) = gitweb_get_feature('ctags');
if ($show_ctags) {
my $ctags = git_get_project_ctags($project);
my $cloud = git_populate_project_tagcloud($ctags);
@@ -4559,7 +4559,7 @@ sub git_blame {
my $fd;
my $ftype;
- gitweb_check_feature('blame')[0]
+ gitweb_get_feature('blame')[0]
or die_error(403, "Blame view not allowed");
die_error(400, "No file name given") unless $file_name;
@@ -4747,7 +4747,7 @@ sub git_blob {
$expires = "+1d";
}
- my ($have_blame) = gitweb_check_feature('blame');
+ my ($have_blame) = gitweb_get_feature('blame');
open my $fd, "-|", git_cmd(), "cat-file", "blob", $hash
or die_error(500, "Couldn't cat $file_name, $hash");
my $mimetype = blob_mimetype($fd, $file_name);
@@ -4840,7 +4840,7 @@ sub git_tree {
my $ref = format_ref_marker($refs, $hash_base);
git_header_html();
my $basedir = '';
- my ($have_blame) = gitweb_check_feature('blame');
+ my ($have_blame) = gitweb_get_feature('blame');
if (defined $hash_base && (my %co = parse_commit($hash_base))) {
my @views_nav = ();
if (defined $file_name) {
@@ -5610,7 +5610,7 @@ sub git_history {
}
sub git_search {
- gitweb_check_feature('search')[0] or die_error(403, "Search is disabled");
+ gitweb_get_feature('search')[0] or die_error(403, "Search is disabled");
if (!defined $searchtext) {
die_error(400, "Text field is empty");
}
@@ -5629,11 +5629,11 @@ sub git_search {
if ($searchtype eq 'pickaxe') {
# pickaxe may take all resources of your box and run for several minutes
# with every query - so decide by yourself how public you make this feature
- gitweb_check_feature('pickaxe')[0]
+ gitweb_get_feature('pickaxe')[0]
or die_error(403, "Pickaxe is disabled");
}
if ($searchtype eq 'grep') {
- gitweb_check_feature('grep')[0]
+ gitweb_get_feature('grep')[0]
or die_error(403, "Grep is disabled");
}
@@ -5838,7 +5838,7 @@ insensitive).</p>
<dt><b>commit</b></dt>
<dd>The commit messages and authorship information will be scanned for the given pattern.</dd>
EOT
- my ($have_grep) = gitweb_check_feature('grep');
+ my ($have_grep) = gitweb_get_feature('grep');
if ($have_grep) {
print <<EOT;
<dt><b>grep</b></dt>
@@ -5855,7 +5855,7 @@ EOT
<dt><b>committer</b></dt>
<dd>Name and e-mail of the committer and date of commit will be scanned for the given pattern.</dd>
EOT
- my ($have_pickaxe) = gitweb_check_feature('pickaxe');
+ my ($have_pickaxe) = gitweb_get_feature('pickaxe');
if ($have_pickaxe) {
print <<EOT;
<dt><b>pickaxe</b></dt>
@@ -5907,7 +5907,7 @@ sub git_shortlog {
sub git_feed {
my $format = shift || 'atom';
- my ($have_blame) = gitweb_check_feature('blame');
+ my ($have_blame) = gitweb_get_feature('blame');
# Atom: http://www.atomenabled.org/developers/syndication/
# RSS: http://www.notestips.com/80256B3A007F2692/1/NAMO5P9UPQ
--
1.6.0.4.850.g6bd829
^ permalink raw reply related
* [PATCH 3/3] gitweb: make gitweb_check_feature a boolean wrapper
From: Junio C Hamano @ 2008-11-29 21:16 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Jakub Narebski, Petr Baudis
In-Reply-To: <7vzljiz1qn.fsf@gitster.siamese.dyndns.org>
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 a 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 add 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.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* Again, this is to demonstrate how I would have liked to see your
patches as a reviewable series. Notice how this naturally justifies
the dropping of parentheses in many lines that begin with "my", and
makes it easier to review? You can review the patch easily by knowing
that callers that have s/get/check/ are now safe to use scalar context.
gitweb/gitweb.perl | 56 +++++++++++++++++++++++++++++++++------------------
1 files changed, 36 insertions(+), 20 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 7ab16cd..acc4cfd 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -190,7 +190,7 @@ our %feature = (
# if there is no 'sub' key (no feature-sub), then feature cannot be
# overriden
#
- # use gitweb_get_feature(<feature>) to check if <feature> is enabled
+ # use gitweb_check_feature(<feature>) to check if <feature> is enabled
# Enable the 'blame' blob view, showing the last commit that modified
# each line in the file. This can be very CPU-intensive.
@@ -344,6 +344,22 @@ sub gitweb_get_feature {
return $sub->(@defaults);
}
+# A wrapper to check if a given feature is enabled.
+# With this, you can say
+#
+# my $bool_feat = gitweb_check_feature('bool_feat');
+# gitweb_check_feature('bool_feat') or somecode;
+#
+# instead of
+#
+# my ($bool_feat) = gitweb_get_feature('bool_feat');
+# (gitweb_get_feature('bool_feat'))[0] or somecode;
+#
+sub gitweb_check_feature {
+ return (gitweb_get_feature(@_))[0];
+}
+
+
sub feature_blame {
my ($val) = git_get_project_config('blame', '--bool');
@@ -810,7 +826,7 @@ sub href (%) {
}
}
- my ($use_pathinfo) = gitweb_get_feature('pathinfo');
+ my $use_pathinfo = gitweb_check_feature('pathinfo');
if ($use_pathinfo) {
# try to put as many parameters as possible in PATH_INFO:
# - project name
@@ -2101,7 +2117,7 @@ sub git_get_projects_list {
$filter ||= '';
$filter =~ s/\.git$//;
- my ($check_forks) = gitweb_get_feature('forks');
+ my $check_forks = gitweb_check_feature('forks');
if (-d $projects_list) {
# search in directory
@@ -2947,7 +2963,7 @@ EOF
}
print "</div>\n";
- my ($have_search) = gitweb_get_feature('search');
+ my $have_search = gitweb_check_feature('search');
if (defined $project && $have_search) {
if (!defined $searchtext) {
$searchtext = "";
@@ -2961,7 +2977,7 @@ EOF
$search_hash = "HEAD";
}
my $action = $my_uri;
- my ($use_pathinfo) = gitweb_get_feature('pathinfo');
+ my $use_pathinfo = gitweb_check_feature('pathinfo');
if ($use_pathinfo) {
$action .= "/".esc_url($project);
}
@@ -3454,7 +3470,7 @@ sub is_patch_split {
sub git_difftree_body {
my ($difftree, $hash, @parents) = @_;
my ($parent) = $parents[0];
- my ($have_blame) = gitweb_get_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
print "<div class=\"list_head\">\n";
if ($#{$difftree} > 10) {
print(($#{$difftree} + 1) . " files changed:\n");
@@ -3914,7 +3930,7 @@ sub fill_project_list_info {
my ($projlist, $check_forks) = @_;
my @projects;
- my ($show_ctags) = gitweb_get_feature('ctags');
+ my $show_ctags = gitweb_check_feature('ctags');
PROJECT:
foreach my $pr (@$projlist) {
my (@activity) = git_get_last_activity($pr->{'path'});
@@ -3968,7 +3984,7 @@ sub git_project_list_body {
# actually uses global variable $project
my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
- my ($check_forks) = gitweb_get_feature('forks');
+ my $check_forks = gitweb_check_feature('forks');
my @projects = fill_project_list_info($projlist, $check_forks);
$order ||= $default_projects_order;
@@ -3988,7 +4004,7 @@ sub git_project_list_body {
@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;
}
- my ($show_ctags) = gitweb_get_feature('ctags');
+ my $show_ctags = gitweb_check_feature('ctags');
if ($show_ctags) {
my %ctags;
foreach my $p (@projects) {
@@ -4428,7 +4444,7 @@ sub git_summary {
my @taglist = git_get_tags_list(16);
my @headlist = git_get_heads_list(16);
my @forklist;
- my ($check_forks) = gitweb_get_feature('forks');
+ my $check_forks = gitweb_check_feature('forks');
if ($check_forks) {
@forklist = git_get_projects_list($project);
@@ -4457,7 +4473,7 @@ sub git_summary {
}
# Tag cloud
- my ($show_ctags) = gitweb_get_feature('ctags');
+ my $show_ctags = gitweb_check_feature('ctags');
if ($show_ctags) {
my $ctags = git_get_project_ctags($project);
my $cloud = git_populate_project_tagcloud($ctags);
@@ -4559,7 +4575,7 @@ sub git_blame {
my $fd;
my $ftype;
- gitweb_get_feature('blame')[0]
+ gitweb_check_feature('blame')
or die_error(403, "Blame view not allowed");
die_error(400, "No file name given") unless $file_name;
@@ -4747,7 +4763,7 @@ sub git_blob {
$expires = "+1d";
}
- my ($have_blame) = gitweb_get_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
open my $fd, "-|", git_cmd(), "cat-file", "blob", $hash
or die_error(500, "Couldn't cat $file_name, $hash");
my $mimetype = blob_mimetype($fd, $file_name);
@@ -4840,7 +4856,7 @@ sub git_tree {
my $ref = format_ref_marker($refs, $hash_base);
git_header_html();
my $basedir = '';
- my ($have_blame) = gitweb_get_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
if (defined $hash_base && (my %co = parse_commit($hash_base))) {
my @views_nav = ();
if (defined $file_name) {
@@ -5610,7 +5626,7 @@ sub git_history {
}
sub git_search {
- gitweb_get_feature('search')[0] or die_error(403, "Search is disabled");
+ gitweb_check_feature('search') or die_error(403, "Search is disabled");
if (!defined $searchtext) {
die_error(400, "Text field is empty");
}
@@ -5629,11 +5645,11 @@ sub git_search {
if ($searchtype eq 'pickaxe') {
# pickaxe may take all resources of your box and run for several minutes
# with every query - so decide by yourself how public you make this feature
- gitweb_get_feature('pickaxe')[0]
+ gitweb_check_feature('pickaxe')
or die_error(403, "Pickaxe is disabled");
}
if ($searchtype eq 'grep') {
- gitweb_get_feature('grep')[0]
+ gitweb_check_feature('grep')
or die_error(403, "Grep is disabled");
}
@@ -5838,7 +5854,7 @@ insensitive).</p>
<dt><b>commit</b></dt>
<dd>The commit messages and authorship information will be scanned for the given pattern.</dd>
EOT
- my ($have_grep) = gitweb_get_feature('grep');
+ my $have_grep = gitweb_check_feature('grep');
if ($have_grep) {
print <<EOT;
<dt><b>grep</b></dt>
@@ -5855,7 +5871,7 @@ EOT
<dt><b>committer</b></dt>
<dd>Name and e-mail of the committer and date of commit will be scanned for the given pattern.</dd>
EOT
- my ($have_pickaxe) = gitweb_get_feature('pickaxe');
+ my $have_pickaxe = gitweb_check_feature('pickaxe');
if ($have_pickaxe) {
print <<EOT;
<dt><b>pickaxe</b></dt>
@@ -5907,7 +5923,7 @@ sub git_shortlog {
sub git_feed {
my $format = shift || 'atom';
- my ($have_blame) = gitweb_get_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
# Atom: http://www.atomenabled.org/developers/syndication/
# RSS: http://www.notestips.com/80256B3A007F2692/1/NAMO5P9UPQ
--
1.6.0.4.850.g6bd829
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox