* Re: [JGIT PATCH 2/2] Add getPatchText functions to obtain the plain-text version of a patch
From: Robin Rosenberg @ 2008-12-13 21:26 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <200812131202.07717.robin.rosenberg.lists@dewire.com>
lördag 13 december 2008 12:02:07 skrev Robin Rosenberg:
> lördag 13 december 2008 03:42:26 skrev Shawn O. Pearce:
> > The conversion from byte[] to String is performed one line at a time,
> > in case the patch is a character encoding conversion patch for the
> > file. For simplicity we currently assume UTF-8 still as the default
> > encoding for any content, but eventually we should support using the
> > .gitattributes encoding property when performing this conversion.
>
> For usefulness we must be able to pass the encoding from outside,
> e.g. the encoding Eclipse uses, which often is not UTF-8-
It's even worse. You should probably do the encoding guess on the whole
patch, or per file and not per line so make success possible at all. Reading
and writing as ISO-8859-1 will always work as that is just padding every
byte with NUL on reading and dropping it on writing. I.e. if your convert
to char at all...
-- robin
^ permalink raw reply
* Re: [PATCH try 2] gitweb: Add option to put a trailing slash on pathinfo-style project URLs
From: Jakub Narebski @ 2008-12-13 21:47 UTC (permalink / raw)
To: Matt McCutchen; +Cc: git, Giuseppe Bilotta
In-Reply-To: <1229202689.31181.1.camel@mattlaptop2.local>
Matt McCutchen <matt@mattmccutchen.net> writes:
> My Web site uses pathinfo mode and some rewrite magic to show the gitweb
> interface at the URL of the real repository directory (which users also
> pull from). In this case, it's desirable to end generated links to the
> project in a trailing slash so the Web server doesn't have to redirect
> the client to add the slash. This patch adds a second element to the
> "pathinfo" feature configuration to control the trailing slash.
>
> Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
Did you check that it does not confuse gitweb if filename parameter is
passed using pathinfo? Gitweb used to rely on final '/' to
distinguish directory pathnames from ordinary pathnames, but I think
currently thanks to the fact that gitweb now embeds action in pathinfo
URL, and does not need to guess type, it is not an issue.
Or only project URLs (i.e. only with project parameter, i.e. only
"http://git.example.com/project.git/" but not other path_info links)
have trailing slash added?
Errr... I see that it adds trailing slash only for project-only
path_info links, but the commit message was not entirely clear for me.
(CC-ed author of path_info enhancements, Giuseppe Bilotta)
> ---
> Resending with a sign-off.
Thanks.
> gitweb/gitweb.perl | 28 ++++++++++++++++++++++------
> 1 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 6eb370d..86511cf 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -270,6 +270,11 @@ our %feature = (
> # $feature{'pathinfo'}{'default'} = [1];
> # Project specific override is not supported.
>
> + # If you want a trailing slash on the project path (because, for
> + # example, you have a real directory at that URL and are using
> + # some rewrite magic to invoke gitweb), then set:
> + # $feature{'pathinfo'}{'default'} = [1, 1];
> +
Are any disadvantages to having it always enabled?
BTW. encoding data in position in array feels a bit hacky to me, but
I guess that is the limitation of current %feature design, with
'default' having to be array (reference).
> # Note that you will need to change the default location of CSS,
> # favicon, logo and possibly other files to an absolute URL. Also,
> # if gitweb.cgi serves as your indexfile, you will need to force
> @@ -829,8 +834,8 @@ sub href (%) {
> }
> }
>
> - my $use_pathinfo = gitweb_check_feature('pathinfo');
> - if ($use_pathinfo) {
> + my @use_pathinfo = gitweb_get_feature('pathinfo');
Why not name those variables for better readability?
+ my ($use_pathinfo, $trailing_slash) = gitweb_get_feature('pathinfo');
> + if ($use_pathinfo[0]) {
> # try to put as many parameters as possible in PATH_INFO:
> # - project name
> # - action
> @@ -845,7 +850,12 @@ sub href (%) {
> $href =~ s,/$,,;
>
> # Then add the project name, if present
> - $href .= "/".esc_url($params{'project'}) if defined $params{'project'};
> + my $proj_href = undef;
> + if (defined $params{'project'}) {
> + $href .= "/".esc_url($params{'project'});
> + # Save for trailing-slash check below.
> + $proj_href = $href;
> + }
> delete $params{'project'};
>
> # since we destructively absorb parameters, we keep this
> @@ -903,6 +913,10 @@ sub href (%) {
> $href .= $known_snapshot_formats{$fmt}{'suffix'};
> delete $params{'snapshot_format'};
> }
> +
> + # If requested in the configuration, add a trailing slash to a URL that
> + # has nothing appended after the project path.
> + $href .= '/' if ($use_pathinfo[1] && defined $proj_href && $href eq $proj_href);
> }
The check _feels_ inefficient. I think (but feel free to disagree) that
it would be better to use something like $project_pathinfo, set it
when adding project as pathinfo, and unset if we add anything else as
pathinfo.
>
> # now encode the parameters explicitly
> @@ -2987,13 +3001,15 @@ EOF
> $search_hash = "HEAD";
> }
> my $action = $my_uri;
> - my $use_pathinfo = gitweb_check_feature('pathinfo');
> - if ($use_pathinfo) {
> + my @use_pathinfo = gitweb_get_feature('pathinfo');
Same comment as above: better named variable instead of relying on
position in array.
> + if ($use_pathinfo[0]) {
> $action .= "/".esc_url($project);
> + # Add a trailing slash if requested in the configuration.
> + $action .= '/' if ($use_pathinfo[1]);
Hmmm... let me check something... you rely on the fact that $project
doesn't end with slash, while I think (but please check it) that it
can end with slash if it is provided by CGI query.
> }
> print $cgi->startform(-method => "get", -action => $action) .
> "<div class=\"search\">\n" .
> - (!$use_pathinfo &&
> + (!$use_pathinfo[0] &&
> $cgi->input({-name=>"p", -value=>$project, -type=>"hidden"}) . "\n") .
> $cgi->input({-name=>"a", -value=>"search", -type=>"hidden"}) . "\n" .
> $cgi->input({-name=>"h", -value=>$search_hash, -type=>"hidden"}) . "\n" .
> --
> 1.6.1.rc2.27.gc7114
>
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: Optimizing cloning of a high object count repository
From: Nicolas Pitre @ 2008-12-13 21:50 UTC (permalink / raw)
To: Resul Cetin; +Cc: git, Nguyen Thai Ngoc Duy, gentoo-scm
In-Reply-To: <alpine.LFD.2.00.0812131347130.30035@xanadu.home>
On Sat, 13 Dec 2008, Nicolas Pitre wrote:
> On Sat, 13 Dec 2008, Resul Cetin wrote:
>
> > On Saturday 13 December 2008 16:46:50 you wrote:
> > [...]
> > > > The size of the linux repository seems to be smaller but in the same
> > > > range object count and repository size but clones are much much faster.
> > > > Is there any way to optimize the server operations like counting and
> > > > compressing of objects to get the same speed as we get from
> > > > git.kernel.org (which does it in nearly no time and the only limiting
> > > > factor seems to be my bandwith)?
> > > > The only other information I have is that Robin H. Johnson made a single
> > > > ~910MiB pack for the whole repository.
> > >
> > > Make yearly packed repository snapshots and publish them via http.
> > > People can wget the latest snapshot, then pull updates later.
> > That would be a workaround but it doesn't explain why git.kernel.org deliveres
> > torvalds repository without any notable counting and compressing time. Maybe
> > it has something todo with the config I found inside the repository:
> > http://git.overlays.gentoo.org/gitroot/exp/gentoo-x86.git/config
> > It says that it isnt a bare repository.
>
> That's not relevant.
>
> The counting time is a bit unfortunate (although I have plans to speed
> that up, if only I can find the time).
>
> You should be able to skip the compression time entirely though, if you
> do repack the repository first. And you want it to be as tightly packed
> as possible for public access. I'm currently cloning it and the
> counting phase is not _that_ bad compared to the compression phase. Try
> something like 'git repack -a -f -d --window=200' and let it run
> overnight if necessary. You need to do this only once, and preferably
> on a machine with lots of RAM, and preferably on a 64-bit machine. Once
> this is done then things should go much more smoothly afterwards.
FYI, I repacked that repository after cloning it, and that operation
required around 2.5G of resident memory. Given the address space
fragmentation, it is possible that a full repack cannot be performed on
a 32-bit machine.
I did 'git repack -a -f -d --window=500 --depth=100'. This took less
than an hour on a quad core machine. The resulting pack is 695MB in
size. That's the amount of data that would be transfered during a
clone of this repository, and nothing would have to be compressed during
the clone as everything is already fully compressed.
Nicolas
^ permalink raw reply
* Re: [PATCH 1/2] gitweb: allow access to forks with strict_export
From: Jakub Narebski @ 2008-12-13 21:53 UTC (permalink / raw)
To: Matt McCutchen; +Cc: git, Petr Baudis
In-Reply-To: <1229203014.31181.7.camel@mattlaptop2.local>
Matt McCutchen <matt@mattmccutchen.net> writes:
CC-ed Petr Baudis, author of forks support in gitweb.
> git_get_projects_list excludes forks in order to unclutter the main
> project list, but this caused the strict_export check, which also relies
> on git_get_project_list, to incorrectly fail for forks. This patch adds
> an argument so git_get_projects_list knows when it is being called for a
> strict_export check (as opposed to a user-visible project list) and
> doesn't exclude the forks.
>
> Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
Looks good for me.
Acked-by: Jakub Narebski <jnareb@gmail.com>
> ---
> gitweb/gitweb.perl | 7 ++++---
> 1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 86511cf..5357bcc 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1144,7 +1144,8 @@ sub untabify {
>
> sub project_in_list {
> my $project = shift;
> - my @list = git_get_projects_list();
> + # Tell git_get_projects_list to include forks.
> + my @list = git_get_projects_list(undef, 1);
> return @list && scalar(grep { $_->{'path'} eq $project } @list);
> }
>
> @@ -2128,13 +2129,13 @@ sub git_get_project_url_list {
> }
>
> sub git_get_projects_list {
> - my ($filter) = @_;
> + my ($filter, $for_strict_export) = @_;
> my @list;
>
> $filter ||= '';
> $filter =~ s/\.git$//;
>
> - my $check_forks = gitweb_check_feature('forks');
> + my $check_forks = !$for_strict_export && gitweb_check_feature('forks');
>
> if (-d $projects_list) {
> # search in directory
> --
> 1.6.1.rc2.27.gc7114
>
>
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH 2/2] gitweb: support hiding projects from user-visible lists
From: Jakub Narebski @ 2008-12-13 22:02 UTC (permalink / raw)
To: Matt McCutchen; +Cc: git
In-Reply-To: <1229203007.31181.6.camel@mattlaptop2.local>
Matt McCutchen <matt@mattmccutchen.net> writes:
Commit message, please?
> Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
> ---
>
> My Web site has a single gitweb installation in which some of the
> repositories are protected by a basic authentication login. By virtue
> of my aforementioned setup with gitweb and pulling at the same URL, the
> login applies uniformly to both. I had to include these repositories in
> the projects_list because I use strict_export, but I want to hide them
> when the user views the project list. This patch implements that
> feature, and the previous one fixes a bug I noticed along the way.
>
> Matt
Cannot you do this with new $export_auth_hook gitweb configuration
variable, added by Alexander Gavrilov in
dd7f5f1 (gitweb: Add a per-repository authorization hook.)
It is used in check_export_ok subroutine, and is is checked also when
getting list of project from file
>From gitweb/INSTALL
- Finally, it is possible to specify an arbitrary perl subroutine that
will be called for each project to determine if it can be exported.
The subroutine receives an absolute path to the project as its only
parameter.
For example, if you use mod_perl to run the script, and have dumb
http protocol authentication configured for your repositories, you
can use the following hook to allow access only if the user is
authorized to read the files:
$export_auth_hook = sub {
use Apache2::SubRequest ();
use Apache2::Const -compile => qw(HTTP_OK);
my $path = "$_[0]/HEAD";
my $r = Apache2::RequestUtil->request;
my $sub = $r->lookup_file($path);
return $sub->filename eq $path
&& $sub->status == Apache2::Const::HTTP_OK;
};
> gitweb/gitweb.perl | 13 +++++++++----
> 1 files changed, 9 insertions(+), 4 deletions(-)
No documentation, in gitweb/README or gitweb/INSTALL
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 5357bcc..085cc60 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1144,7 +1144,7 @@ sub untabify {
>
> sub project_in_list {
> my $project = shift;
> - # Tell git_get_projects_list to include forks.
> + # Tell git_get_projects_list to include forks and hidden repositories.
> my @list = git_get_projects_list(undef, 1);
> return @list && scalar(grep { $_->{'path'} eq $project } @list);
> }
> @@ -2174,15 +2174,18 @@ sub git_get_projects_list {
> # 'git%2Fgit.git Linus+Torvalds'
> # 'libs%2Fklibc%2Fklibc.git H.+Peter+Anvin'
> # 'linux%2Fhotplug%2Fudev.git Greg+Kroah-Hartman'
> + #
> + # 1 in the third field hides the project from user-visible lists, e.g.:
> + # 'linux%2Fembargoed-security-fixes.git John+Doe 1'
I guess I'd rather use _last_ field, in the event we add project
description to project list file format.
> my %paths;
> open my ($fd), $projects_list or return;
> PROJECT:
> while (my $line = <$fd>) {
> chomp $line;
> - my ($path, $owner) = split ' ', $line;
> + my ($path, $owner, $hidden) = split ' ', $line;
> $path = unescape($path);
> $owner = unescape($owner);
> - if (!defined $path) {
> + if (!defined $path || ($hidden && !$for_strict_export)) {
> next;
> }
> if ($filter ne '') {
> @@ -2227,6 +2230,8 @@ sub git_get_projects_list {
> return @list;
> }
>
> +# This is used to look up the owner of a project the user is already allowed to
> +# see, so we shouldn't omit hidden repositories.
> our $gitweb_project_owner = undef;
> sub git_get_project_list_from_file {
>
> @@ -2241,7 +2246,7 @@ sub git_get_project_list_from_file {
> open (my $fd , $projects_list);
> while (my $line = <$fd>) {
> chomp $line;
> - my ($pr, $ow) = split ' ', $line;
> + my ($pr, $ow, $hidden) = split ' ', $line;
> $pr = unescape($pr);
> $ow = unescape($ow);
> $gitweb_project_owner->{$pr} = to_utf8($ow);
> --
> 1.6.1.rc2.27.gc7114
>
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH 2/2] gitweb: support hiding projects from user-visible lists
From: Jakub Narebski @ 2008-12-13 22:05 UTC (permalink / raw)
To: Matt McCutchen; +Cc: git
In-Reply-To: <m3ljujg2eh.fsf@localhost.localdomain>
By the way, your message [PATCH 2/2] should be threaded, i.e. be
response to [PATCH 1/2] (or to cover letter [PATCH 0/2]), to not
mistake it with other [PATCH 2/2] patches.
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: gitweb and unicode special characters
From: Jakub Narebski @ 2008-12-13 22:08 UTC (permalink / raw)
To: Edward Z. Yang; +Cc: git
In-Reply-To: <ghv8rf$47v$1@ger.gmane.org>
"Edward Z. Yang" <edwardzyang@thewritingpot.com> writes:
> Jakub Narebski wrote:
> > Sidenote: There is probably one exception we want to add, namely not
> > escape '\r' at the end of line, to be able to deal better with DOS
> > line endings (\r\n).
>
> I'm sorry, but I have to disagree. I find being able to see \r
> line-endings in the pretty-printed format is exceedingly useful for
> figuring out if a file has been checked in with the wrong line-endings.
> The number of files that must have \r line endings are vanishingly small
> (BAT files are perhaps the one example I can think of right now).
Well, it is a bit annoying if you have checked file with wrong line
endings, and just noticed this... I was thinking about adding '(DOS)'
or something indicator at the bottom of 'blob' and 'blame' views, but
I guess I can live with '\r'...
In short: I agree, that was not a good idea.
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: Git weekly news: 2008-50
From: Jakub Narebski @ 2008-12-13 22:18 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git list
In-Reply-To: <94a0d4530812121710u7755b8a9m87dd134b0c8b04ea@mail.gmail.com>
"Felipe Contreras" <felipe.contreras@gmail.com> writes:
> This week I'm trying to address some of the issues mentioned here. I
> still would like people to request user accounts to this blog if they
> wish to make some git related posts.
>
> http://gitlog.wordpress.com/2008/12/13/git-weekly-links-2008-50/
>
> == Articles ==
>
> Why Subversion does not suck
> http://blog.assembla.com/assemblablog/tabid/12618/bid/7437/Why-Subversion-does-not-suck.aspx
[...]
Thanks a lot. I quite like the new format, both HTML version on blog,
and the format used here in this email.
P.S. Small nitpick: you have changed the title of blog post, but not
subject of email.
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH try 2] gitweb: Add option to put a trailing slash on pathinfo-style project URLs
From: Giuseppe Bilotta @ 2008-12-13 22:23 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Matt McCutchen, git
In-Reply-To: <m3tz97g329.fsf@localhost.localdomain>
On Sat, Dec 13, 2008 at 10:47 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> Matt McCutchen <matt@mattmccutchen.net> writes:
>
>> My Web site uses pathinfo mode and some rewrite magic to show the gitweb
>> interface at the URL of the real repository directory (which users also
>> pull from). In this case, it's desirable to end generated links to the
>> project in a trailing slash so the Web server doesn't have to redirect
>> the client to add the slash. This patch adds a second element to the
>> "pathinfo" feature configuration to control the trailing slash.
>>
>> Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
>
> Did you check that it does not confuse gitweb if filename parameter is
> passed using pathinfo? Gitweb used to rely on final '/' to
> distinguish directory pathnames from ordinary pathnames, but I think
> currently thanks to the fact that gitweb now embeds action in pathinfo
> URL, and does not need to guess type, it is not an issue.
>
> Or only project URLs (i.e. only with project parameter, i.e. only
> "http://git.example.com/project.git/" but not other path_info links)
> have trailing slash added?
>
> Errr... I see that it adds trailing slash only for project-only
> path_info links, but the commit message was not entirely clear for me.
If indeed the additional / is only asked for in summary view, I think
there's no need for a feature toggle, we can always put it there. If
not, I'm really curious about seeing the rewrite rules (they might
also be worth adding to the gitweb documentation as examples of 'power
usage').
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: [PATCH 1/2] gitweb: allow access to forks with strict_export
From: Junio C Hamano @ 2008-12-13 22:31 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Matt McCutchen, git, Petr Baudis
In-Reply-To: <m3prjvg2st.fsf@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> writes:
> Matt McCutchen <matt@mattmccutchen.net> writes:
>
> CC-ed Petr Baudis, author of forks support in gitweb.
>
>> git_get_projects_list excludes forks in order to unclutter the main
>> project list, but this caused the strict_export check, which also relies
>> on git_get_project_list, to incorrectly fail for forks. This patch adds
>> an argument so git_get_projects_list knows when it is being called for a
>> strict_export check (as opposed to a user-visible project list) and
>> doesn't exclude the forks.
>>
>> Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
>
> Looks good for me.
That sounds like a broken API to me.
At least, please have the decency to not call the extra parameter "for
strict export". I would understand it if the extra parameter is called
"toplevel_only" (or its negation, "include_forks").
IOW, don't name a parameter after the name of one caller that happens to
want an unspecified special semantics, without saying what that special
semantics is. Instead, name it after the special semantics that the
argument triggers.
^ permalink raw reply
* Re: [PATCH try 2] gitweb: Add option to put a trailing slash on pathinfo-style project URLs
From: Junio C Hamano @ 2008-12-13 22:37 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Matt McCutchen, git, Giuseppe Bilotta
In-Reply-To: <m3tz97g329.fsf@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> writes:
>> + # If you want a trailing slash on the project path (because, for
>> + # example, you have a real directory at that URL and are using
>> + # some rewrite magic to invoke gitweb), then set:
>> + # $feature{'pathinfo'}{'default'} = [1, 1];
>> +
>
> Are any disadvantages to having it always enabled?
Good question.
>> + my @use_pathinfo = gitweb_get_feature('pathinfo');
>
> Why not name those variables for better readability?
>
> + my ($use_pathinfo, $trailing_slash) = gitweb_get_feature('pathinfo');
Good suggestion.
^ permalink raw reply
* TortoiseGit
From: Alcides Fonseca @ 2008-12-13 22:47 UTC (permalink / raw)
To: git
^ permalink raw reply
* Re: [PATCH 1/2] gitweb: allow access to forks with strict_export
From: Jakub Narebski @ 2008-12-13 22:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Matt McCutchen, git, Petr Baudis
In-Reply-To: <7vr64b4sib.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> > Matt McCutchen <matt@mattmccutchen.net> writes:
>>
>> CC-ed Petr Baudis, author of forks support in gitweb.
>>
>>> git_get_projects_list excludes forks in order to unclutter the main
>>> project list, but this caused the strict_export check, which also relies
>>> on git_get_project_list, to incorrectly fail for forks. This patch adds
>>> an argument so git_get_projects_list knows when it is being called for a
>>> strict_export check (as opposed to a user-visible project list) and
>>> doesn't exclude the forks.
>>>
>>> Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
>>
>> Looks good for me.
>
> That sounds like a broken API to me.
>
> At least, please have the decency to not call the extra parameter "for
> strict export". I would understand it if the extra parameter is called
> "toplevel_only" (or its negation, "include_forks").
>
> IOW, don't name a parameter after the name of one caller that happens to
> want an unspecified special semantics, without saying what that special
> semantics is. Instead, name it after the special semantics that the
> argument triggers.
Ahhh... true.
"no_hide" (currently "include_forks") allows us to _not_ passing this
parameter in other places than project_in_list(); undef is falsy.
By the way, doesn't git_project_index and perhaps git_opml also need
this parameter passed to git_get_projects_list?
Then patch subject would change...
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH] Simplified GIT usage guide
From: Nick Andrew @ 2008-12-13 23:05 UTC (permalink / raw)
To: David Howells
Cc: Johannes Schindelin, torvalds, git, linux-kernel, Miklos Vajna
In-Reply-To: <29095.1229109133@redhat.com>
On Fri, Dec 12, 2008 at 07:12:13PM +0000, David Howells wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > So I think that your document might do a good job scaring people away from
> > Git. But I do not believe that your document, especially in the tone it
> > is written, does a good job of helping Git newbies.
>
> Hmmm. So what would you suggest is a good way to write for GIT newbies? Is
> it just that the overview should be canned or drastically simplified?
The way I did it was to start with the directed acyclic graph of
commits, explaining how branches fork the graph and merges join
it. This was presented to people who know subversion, and so they
immediately became aware that there are other ways to manage source
code than in a linear r1 r2 r3 r4 r5. I described tags and branch
heads briefly.
Next up I described the things you'd do with git: add new commits,
create a branch, merge a branch, rebase, tag, push and fetch and
showed what that does with the dag of commits.
Finally I showed the actual commands used to perform those actions.
I didn't get into the object database structure at all (that was
prepared in case I had extra time).
I think a tutorial shouldn't be written in a way that polarises
peoples' opinions or they come to regard git as a "necessary evil".
If the audience is a person who knows nothing about git, that's
hardly a "git hater" and I think the document starts off on the
wrong foot as a result.
Nick.
^ permalink raw reply
* Re: [PATCH] Simplified GIT usage guide
From: Nick Andrew @ 2008-12-13 23:12 UTC (permalink / raw)
To: Jeff Garzik; +Cc: David Howells, torvalds, git, linux-kernel
In-Reply-To: <4942C2D1.4090309@garzik.org>
On Fri, Dec 12, 2008 at 03:00:17PM -0500, Jeff Garzik wrote:
> David Howells wrote:
>> Add a guide to using GIT's simpler features.
>>
>> Signed-off-by: David Howells <dhowells@redhat.com>
>> ---
>>
>> Documentation/git-haters-guide.txt | 1283 ++++++++++++++++++++++++++++++++++++
>> 1 files changed, 1283 insertions(+), 0 deletions(-)
>> create mode 100644 Documentation/git-haters-guide.txt
>
> What do you feel is missing from the Kernel Hackers' Guide to Git? :)
>
> http://linux.yyz.us/git-howto.html
Better advertising, for one :-)
And how to work with different trees in the same local repo.
Nick.
^ permalink raw reply
* [RFC/PATCH v2] gitweb: Incremental blame (proof of concept)
From: Jakub Narebski @ 2008-12-14 0:17 UTC (permalink / raw)
To: git
Cc: Luben Tuikov, Nanako Shiraishi, Petr Baudis, Fredrik Kuivinen,
Jakub Narebski
In-Reply-To: <20081210200908.16899.36727.stgit@localhost.localdomain>
This is tweaked up version of Petr Baudis <pasky@suse.cz> patch, which
in turn was tweaked up version of Fredrik Kuivinen <frekui@gmail.com>'s
proof of concept patch. It adds 'blame_incremental' view, which
incrementally displays line data in blame view using JavaScript (AJAX).
The original patch by Fredrik Kuivinen has been lightly tested in a
couple of browsers (Firefox, Mozilla, Konqueror, Galeon, Opera and IE6).
The next patch by Petr Baudis has been tested in Firefox and Epiphany.
This patch has been lightly tested in Mozilla 1.17.2 and Konqueror
3.5.3.
This patch does not (contrary to the one by Petr Baudis) enable this
view in gitweb: there are no links leading to 'blame_incremental'
action. You would have to generate URL 'by hand' (e.g. changing 'blame'
or 'blob' in gitweb URL to 'blame_incremental'). Having links in gitweb
lead to this new action (e.g. by rewriting them like in previous patch),
if JavaScript is enabled in browser, is left for later.
Like earlier patch by Per Baudis it avoids code duplication, but it goes
one step further and use git_blame_common for ordinary blame view, for
incremental blame, and (which is change from previous patch) for
incremental blame data.
How the 'blame_incremental' view works:
* gitweb generates initial info by putting file contents (from
git-cat-file) together with line numbers in blame table
* then gitweb makes web browser JavaScript engine call startBlame()
function from blame.js
* startBlame() opens connection to 'blame_data' view, which in turn
calls "git blame --incremental" for a file, and streams output of
git-blame to JavaScript (blame.js)
* blame.js updates line info in blame view, coloring it, and updating
progress info; note that it has to use 3 colors to ensure that
different neighbour groups have different styles
* when 'blame_data' ends, and blame.js finishes updating line info,
it fixes colors to match (as far as possible) ordinary 'blame' view,
and updates generating time info.
This code uses http://ajaxpatterns.org/HTTP_Streaming pattern.
It deals with streamed 'blame_data' server error by notifying about them
in the progress info area (just in case).
This patch adds GITWEB_BLAMEJS compile configuration option, and
modifies git-instaweb.sh to take blame.js into account, but it does not
update gitweb/README file (as it is only proof of concept patch). The
code for git-instaweb.sh was taken from Pasky's patch.
While at it, this patch uniquifies td.dark and td.dark2 style: they
differ only in that td.dark2 doesn't have style for :hover.
This patch also adds showing time (in seconds) it took to generate
a page in page footer (based on example code by Pasky), even though
it is independent change, to be able to evaluate incremental blame in
gitweb better. In proper patch series it would be independent commit;
and it probably would provide fallback if Time::HiRes is not available
(by e.g. not showing generating time info), even though this is
unlikely.
Signed-off-by: Fredrik Kuivinen <frekui@gmail.com>
Signed-off-by: Petr Baudis <pasky@suse.cz>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Differences from previous version of patch:
* Fixed copying git-instaweb related changes from Petr Baudis patch;
git-instaweb should not work with 'blame_incremental' view
* Fixed links in blame table in 'blame_incremental' view, and add
support for href(..., -partial_query=>1)
* The title attribute for "Commit" column ("sha1" cells) agrees with
the one used for 'blame' view, meaning using localtime e.g.
'Kay Sievers, 2005-08-07 21:49:46 +0200'
There was a bug in Pasky and Frederik patch here...
* Incremental blame data generation can lead to neighbour groups
blaming the same commit; such groups are now concatenated when
fixing colors to zebra pattern. This mean that 'blame_incremental'
output should match 'blame' view.
* New feature adding author initials below shortened sha1 of commit
was dropped; it was not present in 'blame' view, and it would make
fixing of line grouping mentioned in previous point much more
difficult.
* Use more robust createRequestObject(), using try ... catch,
taken from AJAX article at WikiPedia.
* Better error handling: show big warning with link to 'blame'
view if scripts are disabled, show error if XMLHttpRequest object
cannot be started, show statusText on server returning status != 200
* use deleteCell (DOM HTML) rather than removeChild (DOM Core)
to delete cell(s) below cell spanning multiple rows
* Set 'commits' to empty associative array to mark memory to be freed
* A few improvements to findColorNo and its helper functions
* Remember about Internet Explorer quirk when setting class attr
* Added many comments, changed names of few variables to be more
readable, rename few functions, split off functions, etc.
* A bit of style cleanup: always use block with if, in continued
(broken) lines put operator at the end of line, use literal object
notation "{}" to initialize empty associative array rather than
cryptic "new Object()", use === and !=== instead of == and !=,
always use radix parameter to parseInt (i.e. parseInt(str, 10))
All those changes were recommended by JSLint.
Makefile | 6 +-
git-instaweb.sh | 7 +
gitweb/blame.js | 470 ++++++++++++++++++++++++++++++++++++++++++++++++++++
gitweb/gitweb.css | 27 +++-
gitweb/gitweb.perl | 263 ++++++++++++++++++++---------
5 files changed, 683 insertions(+), 90 deletions(-)
create mode 100644 gitweb/blame.js
diff --git a/Makefile b/Makefile
index 5158197..d2d3fff 100644
--- a/Makefile
+++ b/Makefile
@@ -218,6 +218,7 @@ GITWEB_HOMETEXT = indextext.html
GITWEB_CSS = gitweb.css
GITWEB_LOGO = git-logo.png
GITWEB_FAVICON = git-favicon.png
+GITWEB_BLAMEJS = blame.js
GITWEB_SITE_HEADER =
GITWEB_SITE_FOOTER =
@@ -1209,13 +1210,14 @@ gitweb/gitweb.cgi: gitweb/gitweb.perl
-e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \
-e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \
-e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \
+ -e 's|++GITWEB_BLAMEJS++|$(GITWEB_BLAMEJS)|g' \
-e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
-e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
$< >$@+ && \
chmod +x $@+ && \
mv $@+ $@
-git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css
+git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css gitweb/blame.js
$(QUIET_GEN)$(RM) $@ $@+ && \
sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
-e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
@@ -1224,6 +1226,8 @@ git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css
-e '/@@GITWEB_CGI@@/d' \
-e '/@@GITWEB_CSS@@/r gitweb/gitweb.css' \
-e '/@@GITWEB_CSS@@/d' \
+ -e '/@@GITWEB_BLAMEJS@@/r gitweb/blame.js' \
+ -e '/@@GITWEB_BLAMEJS@@/d' \
-e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
$@.sh > $@+ && \
chmod +x $@+ && \
diff --git a/git-instaweb.sh b/git-instaweb.sh
index 0843372..510789f 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -268,8 +268,15 @@ gitweb_css () {
EOFGITWEB
}
+gitweb_blamejs () {
+ cat > "$1" <<\EOFGITWEB
+@@GITWEB_BLAMEJS@@
+EOFGITWEB
+}
+
gitweb_cgi "$GIT_DIR/gitweb/gitweb.cgi"
gitweb_css "$GIT_DIR/gitweb/gitweb.css"
+gitweb_blamejs "$GIT_DIR/gitweb/blame.js"
case "$httpd" in
*lighttpd*)
diff --git a/gitweb/blame.js b/gitweb/blame.js
new file mode 100644
index 0000000..6288bd1
--- /dev/null
+++ b/gitweb/blame.js
@@ -0,0 +1,470 @@
+// Copyright (C) 2007, Fredrik Kuivinen <frekui@gmail.com>
+
+var DEBUG = 0;
+function debug(str) {
+ if (DEBUG) {
+ alert(str);
+ }
+}
+
+function createRequestObject() {
+ try {
+ return new XMLHttpRequest();
+ } catch(e) {}
+ try {
+ return new ActiveXObject("Msxml2.XMLHTTP");
+ } catch (e) {}
+ try {
+ return new ActiveXObject("Microsoft.XMLHTTP");
+ } catch (e) {}
+
+ debug("XMLHttpRequest not supported");
+ return null;
+}
+
+var http; // XMLHttpRequest object
+var projectUrl; // partial query
+
+// 'commits' is an associative map. It maps SHA1s to Commit objects.
+var commits = {};
+
+function Commit(sha1) {
+ this.sha1 = sha1;
+}
+
+// convert month or day of the month to string, padding it with
+// '0' (zero) to two characters width if necessary, e.g. 2 -> '02'
+function zeroPad(n) {
+ if (n < 10) {
+ return '0' + n;
+ } else {
+ return n.toString();
+ }
+}
+
+// pad number N with nonbreakable spaces on the right, to WIDTH characters
+// example: spacePad(12, 3) == ' 12' (' ' is nonbreakable space)
+function spacePad(n,width) {
+ var scale = 1;
+ var str = '';
+
+ while (width > 1) {
+ scale *= 10;
+ if (n < scale) {
+ str += ' ';
+ }
+ width--;
+ }
+ return str + n;
+}
+
+var blamedLines = 0;
+var totalLines = '???';
+var div_progress_bar;
+var div_progress_info;
+
+// how many lines does a file have, used in progress info
+function countLines() {
+ var table =
+ document.getElementById('blame_table') ||
+ document.getElementsByTagName('table')[0];
+
+ if (table) {
+ return table.getElementsByTagName('tr').length - 1; // for header
+ } else {
+ return '...';
+ }
+}
+
+// update progress info and length (width) of progress bar
+function updateProgressInfo() {
+ if (!div_progress_info) {
+ div_progress_info = document.getElementById('progress_info');
+ }
+ if (!div_progress_bar) {
+ div_progress_bar = document.getElementById('progress_bar');
+ }
+ if (!div_progress_info && !div_progress_bar) {
+ return;
+ }
+
+ var percentage = Math.floor(100.0*blamedLines/totalLines);
+
+ if (div_progress_info) {
+ div_progress_info.innerHTML = blamedLines + ' / ' + totalLines +
+ ' ('+spacePad(percentage,3)+'%)';
+ }
+
+ if (div_progress_bar) {
+ div_progress_bar.setAttribute('style', 'width: '+percentage+'%;');
+ }
+}
+
+// used to extract N from colorN, where N is a number,
+var colorRe = new RegExp('color([0-9]*)');
+
+// return N if <tr class="colorN">, otherwise return null
+// (some browsers require CSS class names to begin with letter)
+function getColorNo(tr) {
+ if (!tr) {
+ return null;
+ }
+ var className = tr.getAttribute('class');
+ if (className) {
+ match = colorRe.exec(className);
+ if (match) {
+ return parseInt(match[1],10);
+ }
+ }
+ return null;
+}
+
+// return one of given possible colors
+// example: chooseColorNoFrom(2, 3) might be 2 or 3
+function chooseColorNoFrom() {
+ // simplest version
+ return arguments[0];
+}
+
+// given two neigbour <tr> elements, find color which would be different
+// from color of both of neighbours; used to 3-color blame table
+function findColorNo(tr_prev, tr_next) {
+ var color_prev = getColorNo(tr_prev);
+ var color_next = getColorNo(tr_next);
+
+
+ // neither of neighbours has color set
+ if (!color_prev && !color_next) {
+ return chooseColorNoFrom(1,2,3);
+ }
+
+ // either both neighbours have the same color,
+ // or only one of neighbours have color set
+ var color;
+ if (color_prev == color_next) {
+ color = color_prev; // = color_next;
+ } else if (!color_prev) {
+ color = color_next;
+ } else if (!color_next) {
+ color = color_prev;
+ }
+ if (color) {
+ return chooseColorNoFrom((color % 3) + 1, ((color+1) % 3) + 1);
+ }
+
+ // neighbours have different colors
+ return (3 - ((color_prev + color_next) % 3));
+}
+
+// used to extract hours and minutes from timezone info, e.g '-0900'
+var tzRe = new RegExp('^([+-][0-9][0-9])([0-9][0-9])$');
+
+// called for each blame entry, as soon as it finishes
+function handleLine(commit) {
+ /*
+ This is the structure of the HTML fragment we are working
+ with:
+
+ <tr id="l123" class="">
+ <td class="sha1" title=""><a href=""></a></td>
+ <td class="linenr"><a class="linenr" href="">123</a></td>
+ <td class="pre"># times (my ext3 doesn't).</td>
+ </tr>
+ */
+
+ var resline = commit.resline;
+
+ // format date and time string only once per commit
+ if (!commit.info) {
+ var localDate = new Date(); // date corrected by timezone
+ var match = tzRe.exec(commit.authorTimezone);
+ localDate.setTime(1000 * (commit.authorTime +
+ (parseInt(match[1],10)*3600 + parseInt(match[2],10)*60)));
+ var localDateStr = // e.g. '2005-08-07'
+ localDate.getUTCFullYear() + '-' +
+ zeroPad(localDate.getUTCMonth()+1) + '-' +
+ zeroPad(localDate.getUTCDate());
+ var localTimeStr = // e.g. '21:49:46'
+ zeroPad(localDate.getUTCHours()) + ':' +
+ zeroPad(localDate.getUTCMinutes()) + ':' +
+ zeroPad(localDate.getUTCSeconds());
+
+ /* e.g. 'Kay Sievers, 2005-08-07 21:49:46 +0200' */
+ commit.info = commit.author + ', ' + localDateStr + ' ' +
+ localTimeStr + ' ' + commit.authorTimezone;
+ }
+
+ // color depends on group of lines, not only on blamed commit
+ var colorNo = findColorNo(
+ document.getElementById('l'+(resline-1)),
+ document.getElementById('l'+(resline+commit.numlines))
+ );
+
+
+ for (var i = 0; i < commit.numlines; i++) {
+ var tr = document.getElementById('l'+resline);
+ if (!tr) {
+ debug('tr is null! resline: ' + resline);
+ break;
+ }
+ /*
+ <tr id="l123" class="">
+ <td class="sha1" title=""><a href=""></a></td>
+ <td class="linenr"><a class="linenr" href="">123</a></td>
+ <td class="pre"># times (my ext3 doesn't).</td>
+ </tr>
+ */
+ var td_sha1 = tr.firstChild;
+ var a_sha1 = td_sha1.firstChild;
+ var a_linenr = td_sha1.nextSibling.firstChild;
+
+ /* <tr id="l123" class=""> */
+ if (colorNo !== null) {
+ tr.setAttribute('class', 'color'+colorNo);
+ // Internet Explorer needs this
+ tr.setAttribute('className', 'color'+colorNo);
+ }
+ /* <td class="sha1" title="?" rowspan="?"><a href="?">?</a></td> */
+ if (i === 0) {
+ td_sha1.title = commit.info;
+ td_sha1.setAttribute('rowspan', commit.numlines);
+
+ a_sha1.href = projectUrl + ';a=commit;h=' + commit.sha1;
+ a_sha1.innerHTML = commit.sha1.substr(0, 8);
+
+ } else {
+ //tr.removeChild(td_sha1); // DOM2 Core way
+ tr.deleteCell(0); // DOM2 HTML way
+ }
+
+ /* <td class="linenr"><a class="linenr" href="?">123</a></td> */
+ a_linenr.href = projectUrl + ';a=blame;hb=' + commit.sha1 +
+ ';f=' + commit.filename + '#l' + (commit.srcline + i);
+
+ resline++;
+ blamedLines++;
+
+ //updateProgressInfo();
+ }
+}
+
+function startOfGroup(tr) {
+ return tr.firstChild.getAttribute('class') == 'sha1';
+}
+
+function fixColorsAndGroups() {
+ var colorClasses = ['light2', 'dark2'];
+ var linenum = 1;
+ var tr, prev_group;
+ var colorClass = 0;
+
+ while ((tr = document.getElementById('l'+linenum))) {
+ if (startOfGroup(tr, linenum, document)) {
+ if (prev_group &&
+ prev_group.firstChild.firstChild.href ==
+ tr.firstChild.firstChild.href) {
+ // we have to concatenate groups
+ var rows = prev_group.firstChild.getAttribute('rowspan');
+ // assume that we have rowspan even for rowspan="1"
+ prev_group.firstChild.setAttribute('rowspan',
+ (rows + tr.firstChild.getAttribute('rowspan')));
+ tr.removeChild(tr.firstChild);
+ } else {
+ colorClass = (colorClass + 1) % 2;
+ prev_group = tr;
+ }
+ }
+ tr.setAttribute('class', colorClasses[colorClass]);
+ // Internet Explorer needs this
+ tr.setAttribute('className', colorClasses[colorClass]);
+ linenum++;
+ }
+}
+
+var t_interval_server = '';
+var t0 = new Date();
+
+// write how much it took to generate data, and to run script
+function writeTimeInterval() {
+ var info = document.getElementById('generate_time');
+ if (!info) {
+ return;
+ }
+ var t1 = new Date();
+
+ info.innerHTML += ' + ' +
+ t_interval_server+'s server (blame_data) / ' +
+ (t1.getTime() - t0.getTime())/1000 + 's client (JavaScript)';
+}
+
+// ----------------------------------------------------------------------
+
+var prevDataLength = -1;
+var nextLine = 0;
+var inProgress = false;
+
+var sha1Re = new RegExp('([0-9a-f]{40}) ([0-9]+) ([0-9]+) ([0-9]+)');
+var infoRe = new RegExp('([a-z-]+) ?(.*)');
+var endRe = new RegExp('END ?(.*)');
+var curCommit = new Commit();
+
+var pollTimer = null;
+
+function handleResponse() {
+ debug('handleResp ready: ' + http.readyState +
+ ' respText null?: ' + (http.responseText === null) +
+ ' progress: ' + inProgress);
+
+ if (http.readyState != 4 && http.readyState != 3) {
+ return;
+ }
+
+ // the server returned error
+ if (http.readyState == 3 && http.status != 200) {
+ return;
+ }
+ if (http.readyState == 4 && http.status != 200) {
+ if (!div_progress_info) {
+ div_progress_info = document.getElementById('progress_info');
+ }
+
+ if (div_progress_info) {
+ div_progress_info.setAttribute('class', 'error');
+ // Internet Explorer needs this
+ div_progress_info.setAttribute('className', 'error');
+ div_progress_info.innerHTML = 'Server error: ' +
+ http.status+' - '+(http.statusText || 'Error contacting server');
+ }
+
+ clearInterval(pollTimer);
+ inProgress = false;
+ }
+
+ // In konqueror http.responseText is sometimes null here...
+ if (http.responseText === null) {
+ return;
+ }
+
+ // in case we were called before finished processing
+ if (inProgress) {
+ return;
+ } else {
+ inProgress = true;
+ }
+
+ while (prevDataLength != http.responseText.length) {
+ if (http.readyState == 4 &&
+ prevDataLength == http.responseText.length) {
+ break;
+ }
+
+ prevDataLength = http.responseText.length;
+ var response = http.responseText.substring(nextLine);
+ var lines = response.split('\n');
+ nextLine = nextLine + response.lastIndexOf('\n') + 1;
+ if (response[response.length-1] != '\n') {
+ lines.pop();
+ }
+
+ for (var i = 0; i < lines.length; i++) {
+ var match = sha1Re.exec(lines[i]);
+ if (match) {
+ var sha1 = match[1];
+ var srcline = parseInt(match[2],10);
+ var resline = parseInt(match[3],10);
+ var numlines = parseInt(match[4],10);
+ var c = commits[sha1];
+ if (!c) {
+ c = new Commit(sha1);
+ commits[sha1] = c;
+ }
+
+ c.srcline = srcline;
+ c.resline = resline;
+ c.numlines = numlines;
+ curCommit = c;
+ } else if ((match = infoRe.exec(lines[i]))) {
+ var info = match[1];
+ var data = match[2];
+ if (info == 'filename') {
+ curCommit.filename = data;
+ handleLine(curCommit);
+ updateProgressInfo();
+ } else if (info == 'author') {
+ curCommit.author = data;
+ } else if (info == 'author-time') {
+ curCommit.authorTime = parseInt(data,10);
+ } else if (info == 'author-tz') {
+ curCommit.authorTimezone = data;
+ }
+ } else if ((match = endRe.exec(lines[i]))) {
+ t_interval_server = match[1];
+ debug('END: '+lines[i]);
+ } else if (lines[i] !== '') {
+ debug('malformed line: ' + lines[i]);
+ }
+ }
+ }
+
+ // did we finish work?
+ if (http.readyState == 4 &&
+ prevDataLength == http.responseText.length) {
+ clearInterval(pollTimer);
+
+ fixColorsAndGroups();
+ writeTimeInterval();
+ commits = {}; // free memory
+ }
+
+ inProgress = false;
+}
+
+// ============================================================
+// ------------------------------------------------------------
+
+/*
+ Function: startBlame
+
+ Incrementally update line data in blame_incremental view in gitweb.
+
+ Parameters:
+
+ blamedataUrl - URL to server script generating blame data.
+ bUrl -partial URL to project, used to generate links in blame.
+
+ Comments:
+
+ Called from 'blame_incremental' view after loading table with
+ file contents, a base for blame view.
+*/
+function startBlame(blamedataUrl, bUrl) {
+ debug('startBlame('+blamedataUrl+', '+bUrl+')');
+
+ http = createRequestObject();
+ if (!http) {
+ div_progress_info = document.getElementById('progress_info');
+
+ if (div_progress_info) {
+ div_progress_info.setAttribute('class', 'error');
+ // Internet Explorer needs this
+ div_progress_info.setAttribute('className', 'error');
+ div_progress_info.innerHTML = 'XMLHttpRequest not supported';
+ }
+
+ return;
+ }
+
+ t0 = new Date();
+ projectUrl = bUrl;
+ totalLines = countLines();
+ updateProgressInfo();
+
+ http.open('get', blamedataUrl);
+ http.onreadystatechange = handleResponse;
+ http.send(null);
+
+ // not all browsers call onreadystatechange event on each server flush
+ pollTimer = setInterval(handleResponse, 2000);
+}
+
+// end of blame.js
diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index a01eac8..e359618 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -223,11 +223,7 @@ tr.light:hover {
background-color: #edece6;
}
-tr.dark {
- background-color: #f6f6f0;
-}
-
-tr.dark2 {
+tr.dark, tr.dark2 {
background-color: #f6f6f0;
}
@@ -235,6 +231,14 @@ tr.dark:hover {
background-color: #edece6;
}
+tr.color1:hover { background-color: #e6ede6; }
+tr.color2:hover { background-color: #e6e6ed; }
+tr.color3:hover { background-color: #ede6e6; }
+
+tr.color1 { background-color: #f6fff6; }
+tr.color2 { background-color: #f6f6ff; }
+tr.color3 { background-color: #fff6f6; }
+
td {
padding: 2px 5px;
font-size: 100%;
@@ -255,7 +259,7 @@ td.sha1 {
font-family: monospace;
}
-td.error {
+.error {
color: red;
background-color: yellow;
}
@@ -326,6 +330,17 @@ td.mode {
font-family: monospace;
}
+/* progress of blame_interactive */
+div#progress_bar {
+ height: 2px;
+ margin-bottom: -2px;
+ background-color: #d8d9d0;
+}
+div#progress_info {
+ float: right;
+ text-align: right;
+}
+
/* styling of diffs (patchsets): commitdiff and blobdiff views */
div.diff.header,
div.diff.extended_header {
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4987fdc..93a4e82 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -18,6 +18,9 @@ use File::Find qw();
use File::Basename qw(basename);
binmode STDOUT, ':utf8';
+use Time::HiRes qw(gettimeofday tv_interval);
+our $t0 = [gettimeofday];
+
BEGIN {
CGI->compile() if $ENV{'MOD_PERL'};
}
@@ -74,6 +77,8 @@ our $stylesheet = undef;
our $logo = "++GITWEB_LOGO++";
# URI of GIT favicon, assumed to be image/png type
our $favicon = "++GITWEB_FAVICON++";
+# URI of blame.js
+our $blamejs = "++GITWEB_BLAMEJS++";
# URI and label (title) of GIT logo link
#our $logo_url = "http://www.kernel.org/pub/software/scm/git/docs/";
@@ -493,6 +498,8 @@ our %cgi_param_mapping = @cgi_param_mapping;
# we will also need to know the possible actions, for validation
our %actions = (
"blame" => \&git_blame,
+ "blame_incremental" => \&git_blame_incremental,
+ "blame_data" => \&git_blame_data,
"blobdiff" => \&git_blobdiff,
"blobdiff_plain" => \&git_blobdiff_plain,
"blob" => \&git_blob,
@@ -919,7 +926,8 @@ sub href (%) {
}
}
}
- $href .= "?" . join(';', @result) if scalar @result;
+ $href .= "?" . join(';', @result)
+ if ($params{-partial_query} or scalar @result);
return $href;
}
@@ -1272,7 +1280,7 @@ use constant {
};
# submodule/subproject, a commit object reference
-sub S_ISGITLINK($) {
+sub S_ISGITLINK {
my $mode = shift;
return (($mode & S_IFMT) == S_IFGITLINK)
@@ -1558,7 +1566,7 @@ sub format_diff_from_to_header {
# no extra formatting for "^--- /dev/null"
if (! $diffinfo->{'nparents'}) {
# ordinary (single parent) diff
- if ($line =~ m!^--- "?a/!) {
+ if ($line =~ m!^--- "?a/!) {#"
if ($from->{'href'}) {
$line = '--- a/' .
$cgi->a({-href=>$from->{'href'}, -class=>"path"},
@@ -1816,7 +1824,7 @@ sub git_cmd {
# Try to avoid using this function wherever possible.
sub quote_command {
return join(' ',
- map( { my $a = $_; $a =~ s/(['!])/'\\$1'/g; "'$a'" } @_ ));
+ map( { my $a = $_; $a =~ s/(['!])/'\\$1'/g; "'$a'" } @_ ));#'
}
# get HEAD ref of given project as hash
@@ -2874,13 +2882,13 @@ sub git_header_html {
# 'application/xhtml+xml', otherwise send it as plain old 'text/html'.
# we have to do this because MSIE sometimes globs '*/*', pretending to
# support xhtml+xml but choking when it gets what it asked for.
- if (defined $cgi->http('HTTP_ACCEPT') &&
- $cgi->http('HTTP_ACCEPT') =~ m/(,|;|\s|^)application\/xhtml\+xml(,|;|\s|$)/ &&
- $cgi->Accept('application/xhtml+xml') != 0) {
- $content_type = 'application/xhtml+xml';
- } else {
+ #if (defined $cgi->http('HTTP_ACCEPT') &&
+ # $cgi->http('HTTP_ACCEPT') =~ m/(,|;|\s|^)application\/xhtml\+xml(,|;|\s|$)/ &&
+ # $cgi->Accept('application/xhtml+xml') != 0) {
+ # $content_type = 'application/xhtml+xml';
+ #} else {
$content_type = 'text/html';
- }
+ #}
print $cgi->header(-type=>$content_type, -charset => 'utf-8',
-status=> $status, -expires => $expires);
my $mod_perl_version = $ENV{'MOD_PERL'} ? " $ENV{'MOD_PERL'}" : '';
@@ -3042,6 +3050,14 @@ sub git_footer_html {
}
print "</div>\n"; # class="page_footer"
+ print "<div class=\"page_footer\">\n";
+ print 'This page took '.
+ '<span id="generate_time" class="time_span">'.
+ tv_interval($t0, [gettimeofday]).'s'.
+ '</span>'.
+ " to generate.\n";
+ print "</div>\n"; # class="page_footer"
+
if (-f $site_footer) {
insert_file($site_footer);
}
@@ -3803,7 +3819,7 @@ sub git_patchset_body {
while ($patch_line) {
# parse "git diff" header line
- if ($patch_line =~ m/^diff --git (\"(?:[^\\\"]*(?:\\.[^\\\"]*)*)\"|[^ "]*) (.*)$/) {
+ if ($patch_line =~ m/^diff --git (\"(?:[^\\\"]*(?:\\.[^\\\"]*)*)\"|[^ "]*) (.*)$/) {#"
# $1 is from_name, which we do not use
$to_name = unquote($2);
$to_name =~ s!^b/!!;
@@ -4574,7 +4590,9 @@ sub git_tag {
git_footer_html();
}
-sub git_blame {
+sub git_blame_common {
+ my $format = shift || 'porcelain';
+
# permissions
gitweb_check_feature('blame')
or die_error(403, "Blame view not allowed");
@@ -4596,10 +4614,36 @@ sub git_blame {
}
}
- # run git-blame --porcelain
- open my $fd, "-|", git_cmd(), "blame", '-p',
- $hash_base, '--', $file_name
- or die_error(500, "Open git-blame failed");
+ my $fd;
+ if ($format eq 'incremental') {
+ # get file contents (as base)
+ open $fd, "-|", git_cmd(), 'cat-file', 'blob', $hash
+ or die_error(500, "Open git-cat-file failed");
+ } elsif ($format eq 'data') {
+ # run git-blame --incremental
+ open $fd, "-|", git_cmd(), "blame", "--incremental",
+ $hash_base, "--", $file_name
+ or die_error(500, "Open git-blame --incremental failed");
+ } else {
+ # run git-blame --porcelain
+ open $fd, "-|", git_cmd(), "blame", '-p',
+ $hash_base, '--', $file_name
+ or die_error(500, "Open git-blame --porcelain failed");
+ }
+
+ # incremental blame data returns early
+ if ($format eq 'data') {
+ print $cgi->header(
+ -type=>"text/plain", -charset => "utf-8",
+ -status=> "200 OK");
+ local $| = 1; # output autoflush
+ print while <$fd>;
+ close $fd
+ or print "ERROR $!\n";
+ print "END ".tv_interval($t0, [gettimeofday])."\n";
+
+ return;
+ }
# page header
git_header_html();
@@ -4610,93 +4654,146 @@ sub git_blame {
$cgi->a({-href => href(action=>"history", -replay=>1)},
"history") .
" | " .
- $cgi->a({-href => href(action=>"blame", file_name=>$file_name)},
+ $cgi->a({-href => href(action=>$action, file_name=>$file_name)},
"HEAD");
git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
git_print_page_path($file_name, $ftype, $hash_base);
# page body
+ if ($format eq 'incremental') {
+ print "<noscript>\n<div class=\"error\"><center><b>\n".
+ "This page requires JavaScript to run\nUse ".
+ $cgi->a({-href => href(action=>'blame',-replay=>1)}, 'this page').
+ " instead.\n".
+ "</b></center></div>\n</noscript>\n";
+
+ print qq!<div id="progress_bar" style="width: 100%; background-color: yellow"></div>\n!;
+ }
+
+ print qq!<div class="page_body">\n!;
+ print qq!<div id="progress_info">... / ...</div>\n!
+ if ($format eq 'incremental');
+ print qq!<table id="blame_table" class="blame" width="100%">\n!.
+ #qq!<col width="5.5em" /><col width="2.5em" /><col width="*" />\n!.
+ qq!<thead>\n!.
+ qq!<tr><th>Commit</th><th>Line</th><th>Data</th></tr>\n!.
+ qq!</thead>\n!.
+ qq!<tbody>\n!;
+
my @rev_color = qw(light2 dark2);
my $num_colors = scalar(@rev_color);
my $current_color = 0;
- my %metainfo = ();
- print <<HTML;
-<div class="page_body">
-<table class="blame">
-<tr><th>Commit</th><th>Line</th><th>Data</th></tr>
-HTML
- LINE:
- while (my $line = <$fd>) {
- chomp $line;
- # the header: <SHA-1> <src lineno> <dst lineno> [<lines in group>]
- # no <lines in group> for subsequent lines in group of lines
- my ($full_rev, $orig_lineno, $lineno, $group_size) =
- ($line =~ /^([0-9a-f]{40}) (\d+) (\d+)(?: (\d+))?$/);
- if (!exists $metainfo{$full_rev}) {
- $metainfo{$full_rev} = {};
- }
- my $meta = $metainfo{$full_rev};
- my $data; # last line is used later
- while ($data = <$fd>) {
- chomp $data;
- last if ($data =~ s/^\t//); # contents of line
- if ($data =~ /^(\S+) (.*)$/) {
- $meta->{$1} = $2;
- }
- }
- my $short_rev = substr($full_rev, 0, 8);
- my $author = $meta->{'author'};
- my %date =
- parse_date($meta->{'author-time'}, $meta->{'author-tz'});
- my $date = $date{'iso-tz'};
- if ($group_size) {
- $current_color = ($current_color + 1) % $num_colors;
- }
- print "<tr id=\"l$lineno\" class=\"$rev_color[$current_color]\">\n";
- if ($group_size) {
- print "<td class=\"sha1\"";
- print " title=\"". esc_html($author) . ", $date\"";
- print " rowspan=\"$group_size\"" if ($group_size > 1);
- print ">";
- print $cgi->a({-href => href(action=>"commit",
- hash=>$full_rev,
- file_name=>$file_name)},
- esc_html($short_rev));
- print "</td>\n";
+ if ($format eq 'incremental') {
+ my $color_class = $rev_color[$current_color];
+
+ #contents of a file
+ my $linenr = 0;
+ LINE:
+ while (my $line = <$fd>) {
+ chomp $line;
+ $linenr++;
+
+ print qq!<tr id="l$linenr" class="$color_class">!.
+ qq!<td class="sha1"><a href=""></a></td>!.
+ qq!<td class="linenr">!.
+ qq!<a class="linenr" href="">$linenr</a></td>!;
+ print qq!<td class="pre">! . esc_html($line) . "</td>\n";
+ print qq!</tr>\n!;
}
- my $parent_commit;
- if (!exists $meta->{'parent'}) {
- open (my $dd, "-|", git_cmd(), "rev-parse", "$full_rev^")
- or die_error(500, "Open git-rev-parse failed");
- $parent_commit = <$dd>;
- close $dd;
- chomp($parent_commit);
- $meta->{'parent'} = $parent_commit;
- } else {
- $parent_commit = $meta->{'parent'};
- }
- my $blamed = href(action => 'blame',
- file_name => $meta->{'filename'},
- hash_base => $parent_commit);
- print "<td class=\"linenr\">";
- print $cgi->a({ -href => "$blamed#l$orig_lineno",
- -class => "linenr" },
- esc_html($lineno));
- print "</td>";
- print "<td class=\"pre\">" . esc_html($data) . "</td>\n";
- print "</tr>\n";
- }
- print "</table>\n";
- print "</div>";
+
+ } else { # porcelain, i.e. ordinary blame
+ my %metainfo = (); # saves information about commits
+
+ # blame data
+ LINE:
+ while (my $line = <$fd>) {
+ chomp $line;
+ # the header: <SHA-1> <src lineno> <dst lineno> [<lines in group>]
+ # no <lines in group> for subsequent lines in group of lines
+ my ($full_rev, $orig_lineno, $lineno, $group_size) =
+ ($line =~ /^([0-9a-f]{40}) (\d+) (\d+)(?: (\d+))?$/);
+ $metainfo{$full_rev} ||= {};
+ my $meta = $metainfo{$full_rev};
+ my $data; # last line is used later
+ while ($data = <$fd>) {
+ chomp $data;
+ last if ($data =~ s/^\t//); # contents of line
+ if ($data =~ /^(\S+) (.*)$/) {
+ $meta->{$1} = $2;
+ }
+ }
+ my $short_rev = substr($full_rev, 0, 8);
+ my $author = $meta->{'author'};
+ my %date =
+ parse_date($meta->{'author-time'}, $meta->{'author-tz'});
+ my $date = $date{'iso-tz'};
+ if ($group_size) {
+ $current_color = ($current_color + 1) % $num_colors;
+ }
+ print qq!<tr id="l$lineno" class="$rev_color[$current_color]">\n!;
+ if ($group_size) {
+ print qq!<td class="sha1"!.
+ qq! title="!. esc_html($author) . qq!, $date"!;
+ print qq! rowspan="$group_size"! if ($group_size > 1);
+ print qq!>!;
+ print $cgi->a({-href => href(action=>"commit",
+ hash=>$full_rev,
+ file_name=>$file_name)},
+ esc_html($short_rev));
+ print qq!</td>\n!;
+ }
+ if (!exists $meta->{'parent'}) {
+ open my $dd, "-|", git_cmd(), "rev-parse", "$full_rev^"
+ or die_error(500, "Open git-rev-parse failed");
+ $meta->{'parent'} = <$dd>;
+ close $dd;
+ chomp $meta->{'parent'};
+ }
+ my $blamed = href(action => 'blame',
+ file_name => $meta->{'filename'},
+ hash_base => $meta->{'parent'});
+ print qq!<td class="linenr">!.
+ $cgi->a({ -href => "$blamed#l$orig_lineno",
+ -class => "linenr" },
+ esc_html($lineno)).
+ qq!</td>!;
+ print qq!<td class="pre">! . esc_html($data) . "</td>\n";
+ print qq!</tr>\n!;
+ }
+ }
+
+ # footer
+ print "</tbody>\n".
+ "</table>\n"; # class="blame"
+ print "</div>\n"; # class="blame_body"
close $fd
or print "Reading blob failed\n";
- # page footer
+ if ($format eq 'incremental') {
+ print qq!<script type="text/javascript" src="$blamejs"></script>\n!.
+ qq!<script type="text/javascript">\n!.
+ qq!startBlame("!. href(action=>"blame_data", -replay=>1) .qq!",\n!.
+ qq! "!. href(-partial_query=>1) .qq!");\n!.
+ qq!</script>\n!;
+ }
+
git_footer_html();
}
+sub git_blame {
+ git_blame_common();
+}
+
+sub git_blame_incremental {
+ git_blame_common('incremental');
+}
+
+sub git_blame_data {
+ git_blame_common('data');
+}
+
sub git_tags {
my $head = git_get_head_hash($project);
git_header_html();
--
1.6.0.4
^ permalink raw reply related
* Re: [PATCH try 2] gitweb: Add option to put a trailing slash on pathinfo-style project URLs
From: Matt McCutchen @ 2008-12-14 1:13 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: Jakub Narebski, git
In-Reply-To: <cb7bb73a0812131423h1f629ec1n9e8eacd657a4901@mail.gmail.com>
On Sat, 2008-12-13 at 23:23 +0100, Giuseppe Bilotta wrote:
> On Sat, Dec 13, 2008 at 10:47 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> > Matt McCutchen <matt@mattmccutchen.net> writes:
> >
> >> My Web site uses pathinfo mode and some rewrite magic to show the gitweb
> >> interface at the URL of the real repository directory (which users also
> >> pull from). In this case, it's desirable to end generated links to the
> >> project in a trailing slash so the Web server doesn't have to redirect
> >> the client to add the slash. This patch adds a second element to the
> >> "pathinfo" feature configuration to control the trailing slash.
> >>
> >> Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
> >
> > Did you check that it does not confuse gitweb if filename parameter is
> > passed using pathinfo? Gitweb used to rely on final '/' to
> > distinguish directory pathnames from ordinary pathnames, but I think
> > currently thanks to the fact that gitweb now embeds action in pathinfo
> > URL, and does not need to guess type, it is not an issue.
> >
> > Or only project URLs (i.e. only with project parameter, i.e. only
> > "http://git.example.com/project.git/" but not other path_info links)
> > have trailing slash added?
> >
> > Errr... I see that it adds trailing slash only for project-only
> > path_info links, but the commit message was not entirely clear for me.
>
> If indeed the additional / is only asked for in summary view, I think
> there's no need for a feature toggle, we can always put it there. If
> not, I'm really curious about seeing the rewrite rules (they might
> also be worth adding to the gitweb documentation as examples of 'power
> usage').
The trailing slash is used only when the URL refers to a project with no
appended parameters (i.e., summary view), because the URL refers to the
real git dir on disk (hence, pulling from the same URL) and it plays
nicer with the Web server configuration to have the trailing slash.
I was wary about changing the default behavior, but if you and Jakub
both think it's OK, that's great.
I was thinking of proposing the addition of some info about my setup,
including the rewrite rules, to the documentation. Maybe we could do
that after dealing with the patches.
--
Matt
^ permalink raw reply
* Re: [PATCH try 2] gitweb: Add option to put a trailing slash on pathinfo-style project URLs
From: Matt McCutchen @ 2008-12-14 1:43 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Giuseppe Bilotta
In-Reply-To: <m3tz97g329.fsf@localhost.localdomain>
On Sat, 2008-12-13 at 13:47 -0800, Jakub Narebski wrote:
> Errr... I see that it adds trailing slash only for project-only
> path_info links, but the commit message was not entirely clear for me.
I will clarify the message.
> BTW. encoding data in position in array feels a bit hacky to me, but
> I guess that is the limitation of current %feature design, with
> 'default' having to be array (reference).
>
> > # Note that you will need to change the default location of CSS,
> > # favicon, logo and possibly other files to an absolute URL. Also,
> > # if gitweb.cgi serves as your indexfile, you will need to force
> > @@ -829,8 +834,8 @@ sub href (%) {
> > }
> > }
> >
> > - my $use_pathinfo = gitweb_check_feature('pathinfo');
> > - if ($use_pathinfo) {
> > + my @use_pathinfo = gitweb_get_feature('pathinfo');
>
> Why not name those variables for better readability?
>
> + my ($use_pathinfo, $trailing_slash) = gitweb_get_feature('pathinfo');
I'll do that.
> > + if ($use_pathinfo[0]) {
> > # try to put as many parameters as possible in PATH_INFO:
> > # - project name
> > # - action
> > @@ -845,7 +850,12 @@ sub href (%) {
> > $href =~ s,/$,,;
> >
> > # Then add the project name, if present
> > - $href .= "/".esc_url($params{'project'}) if defined $params{'project'};
> > + my $proj_href = undef;
> > + if (defined $params{'project'}) {
> > + $href .= "/".esc_url($params{'project'});
> > + # Save for trailing-slash check below.
> > + $proj_href = $href;
> > + }
> > delete $params{'project'};
> >
> > # since we destructively absorb parameters, we keep this
> > @@ -903,6 +913,10 @@ sub href (%) {
> > $href .= $known_snapshot_formats{$fmt}{'suffix'};
> > delete $params{'snapshot_format'};
> > }
> > +
> > + # If requested in the configuration, add a trailing slash to a URL that
> > + # has nothing appended after the project path.
> > + $href .= '/' if ($use_pathinfo[1] && defined $proj_href && $href eq $proj_href);
> > }
>
> The check _feels_ inefficient. I think (but feel free to disagree) that
> it would be better to use something like $project_pathinfo, set it
> when adding project as pathinfo, and unset if we add anything else as
> pathinfo.
I considered doing that, but I decided that not having to litter the
preceding code with manipulation of $project_pathinfo outweighed
whatever negligible performance difference there might be.
> > + if ($use_pathinfo[0]) {
> > $action .= "/".esc_url($project);
> > + # Add a trailing slash if requested in the configuration.
> > + $action .= '/' if ($use_pathinfo[1]);
>
> Hmmm... let me check something... you rely on the fact that $project
> doesn't end with slash, while I think (but please check it) that it
> can end with slash if it is provided by CGI query.
You are right; in fact, this is already a problem for the strict_export
check. Gitweb should probably strip trailing slashes when it reads the
"p" parameter. I will submit a separate patch for that.
--
Matt
^ permalink raw reply
* Re: [PATCH] Simplified GIT usage guide
From: Ping Yin @ 2008-12-14 1:45 UTC (permalink / raw)
To: Nick Andrew
Cc: David Howells, Johannes Schindelin, torvalds, git, linux-kernel,
Miklos Vajna
In-Reply-To: <20081213230504.GA21912@mail.local.tull.net>
On Sun, Dec 14, 2008 at 7:05 AM, Nick Andrew <nick@nick-andrew.net> wrote:
> The way I did it was to start with the directed acyclic graph of
> commits, explaining how branches fork the graph and merges join
> it. This was presented to people who know subversion, and so they
> immediately became aware that there are other ways to manage source
> code than in a linear r1 r2 r3 r4 r5. I described tags and branch
> heads briefly.
>
> Next up I described the things you'd do with git: add new commits,
> create a branch, merge a branch, rebase, tag, push and fetch and
> showed what that does with the dag of commits.
>
> Finally I showed the actual commands used to perform those actions.
> I didn't get into the object database structure at all (that was
> prepared in case I had extra time).
>
I think this is the right way to start with the DAG. And i do the same.
^ permalink raw reply
* Re: [PATCH 1/2] gitweb: allow access to forks with strict_export
From: Matt McCutchen @ 2008-12-14 1:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jakub Narebski, git, Petr Baudis
In-Reply-To: <7vr64b4sib.fsf@gitster.siamese.dyndns.org>
On Sat, 2008-12-13 at 14:31 -0800, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
> > Matt McCutchen <matt@mattmccutchen.net> writes:
> >
> > CC-ed Petr Baudis, author of forks support in gitweb.
> >
> >> git_get_projects_list excludes forks in order to unclutter the main
> >> project list, but this caused the strict_export check, which also relies
> >> on git_get_project_list, to incorrectly fail for forks. This patch adds
> >> an argument so git_get_projects_list knows when it is being called for a
> >> strict_export check (as opposed to a user-visible project list) and
> >> doesn't exclude the forks.
> >>
> >> Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
> >
> > Looks good for me.
>
> That sounds like a broken API to me.
>
> At least, please have the decency to not call the extra parameter "for
> strict export". I would understand it if the extra parameter is called
> "toplevel_only" (or its negation, "include_forks").
>
> IOW, don't name a parameter after the name of one caller that happens to
> want an unspecified special semantics, without saying what that special
> semantics is. Instead, name it after the special semantics that the
> argument triggers.
I disagree. The parameter is really "include forks (if there is such a
concept under the current config)", and with my second patch, it becomes
"include hidden projects" too. That's really unwieldy.
In my view, the parameter makes the distinction between generating a
filtered list for user consumption and a list of everything for a
strict_export check. The particular semantics it activates may evolve
as gitweb does (case in point: my second patch). The current semantics
can be described in a comment on git_get_projects_list.
Granted, there may be a better name for the parameter than
$for_strict_export. How about $include_all?
--
Matt
^ permalink raw reply
* Re: Git weekly news: 2008-50
From: Jeff Whiteside @ 2008-12-14 2:14 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Felipe Contreras, git list
In-Reply-To: <3ab397d0812131813s5c84aa98kdd76bc646bfb82b0@mail.gmail.com>
You seemed to get a lot of flak lastweek, but I liked the content
(both weeks), especially "why git is > than x" and the github
screencast link. Thanks mate.
On Sat, Dec 13, 2008 at 6:13 PM, Jeff Whiteside
<jeff.m.whiteside@gmail.com> wrote:
>
> You seemed to get a lot of flak lastweek, but I liked the content (both weeks), especially "why git is > than x" and the github screencast link. Thanks mate.
>
> On Sat, Dec 13, 2008 at 2:18 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>>
>> "Felipe Contreras" <felipe.contreras@gmail.com> writes:
>>
>> > This week I'm trying to address some of the issues mentioned here. I
>> > still would like people to request user accounts to this blog if they
>> > wish to make some git related posts.
>> >
>> > http://gitlog.wordpress.com/2008/12/13/git-weekly-links-2008-50/
>> >
>> > == Articles ==
>> >
>> > Why Subversion does not suck
>> > http://blog.assembla.com/assemblablog/tabid/12618/bid/7437/Why-Subversion-does-not-suck.aspx
>> [...]
>>
>> Thanks a lot. I quite like the new format, both HTML version on blog,
>> and the format used here in this email.
>>
>> P.S. Small nitpick: you have changed the title of blog post, but not
>> subject of email.
>>
>> --
>> Jakub Narebski
>> Poland
>> ShadeHawk on #git
>> --
>> 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
>
^ permalink raw reply
* Re: [PATCH 1/2] gitweb: allow access to forks with strict_export
From: Matt McCutchen @ 2008-12-14 2:06 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, git, Petr Baudis
In-Reply-To: <200812132351.37420.jnareb@gmail.com>
On Sat, 2008-12-13 at 23:51 +0100, Jakub Narebski wrote:
> "no_hide" (currently "include_forks") allows us to _not_ passing this
> parameter in other places than project_in_list(); undef is falsy.
Right. That's why I made the current parameter $for_strict_export (so
only project_in_list passes it) rather than the negation.
> By the way, doesn't git_project_index and perhaps git_opml also need
> this parameter passed to git_get_projects_list?
Yes, now that you mention it, I suppose they should show forks, though
not hidden repositories. Then git_get_projects_list can be called in
three different modes: include everything (project_in_list), include
forks but not hidden (git_get_project_index and git_opml), or include
neither forks nor hidden (git_project_list). Should we have two
separate parameters to git_get_projects_list or a single three-valued
one?
That raises another point. I was going to change git_get_projects_list
so that forks of a hidden project that are not themselves hidden appear
on the parent project's page but not in the main project list. This
way, users who know about the parent project can navigate to the fork,
but the fork does not give away the existence of the parent project by
appearing in the main list. Then I guess git_project_index and git_opml
should omit forks of hidden projects, meaning that some fork-checking
still has to take place with "include forks" on but "include hidden"
off. This will make git_get_projects_list somewhat more complex but not
unmanageably so, and I do think it's the behavior we want.
I will send an updated patch.
--
Matt
^ permalink raw reply
* [PATCH] git-fast-import possible memory corruption problem
From: YONETANI Tomokazu @ 2008-12-14 2:08 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 919 bytes --]
Hello.
While trying to convert NetBSD CVS repository to Git, I've been
experiencing 100% reproducible crash of git-fast-import. After
poking here and there and I noticed a dubious code fragment in
pool_alloc():
:
r = p->next_free;
/* round out to a 'uintmax_t' alignment */
if (len & (sizeof(uintmax_t) - 1))
len += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));
p->next_free += len;
return r;
As the `round out' takes place AFTER it found the room in the mem_pool,
there's a small chance of p->next_free being set outside of the chosen
area, up to (sizeof(uintmax_t) - 1) bytes. pool_strdup() is one of the
functions which can trigger the problem, when pool_alloc() finds a room
at the end of a pool entry and the requested length is not multiple of
size(uintmax_t). I believe attached patch addresses this problem.
Best regards,
YONETANI Tomokazu.
[-- Attachment #2: git-fast-import.patch --]
[-- Type: text/plain, Size: 1489 bytes --]
diff --git a/fast-import.c b/fast-import.c
index 3c035a5..fb4367a 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -549,11 +549,15 @@ static unsigned int hc_str(const char *s, size_t len)
return r;
}
-static void *pool_alloc(size_t len)
+static void *_pool_alloc(size_t len, int zero)
{
struct mem_pool *p;
void *r;
+ /* round out to a 'uintmax_t' alignment */
+ if (len & (sizeof(uintmax_t) - 1))
+ len += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));
+
for (p = mem_pool; p; p = p->next_pool)
if ((p->end - p->next_free >= len))
break;
@@ -561,7 +565,8 @@ static void *pool_alloc(size_t len)
if (!p) {
if (len >= (mem_pool_alloc/2)) {
total_allocd += len;
- return xmalloc(len);
+ r = xmalloc(len);
+ goto out;
}
total_allocd += sizeof(struct mem_pool) + mem_pool_alloc;
p = xmalloc(sizeof(struct mem_pool) + mem_pool_alloc);
@@ -572,19 +577,22 @@ static void *pool_alloc(size_t len)
}
r = p->next_free;
- /* round out to a 'uintmax_t' alignment */
- if (len & (sizeof(uintmax_t) - 1))
- len += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));
p->next_free += len;
+out:
+ if (zero)
+ memset(r, 0, len);
return r;
}
+static void *pool_alloc(size_t len)
+{
+ return _pool_alloc(len, 0);
+}
+
static void *pool_calloc(size_t count, size_t size)
{
size_t len = count * size;
- void *r = pool_alloc(len);
- memset(r, 0, len);
- return r;
+ return _pool_alloc(len, 1);
}
static char *pool_strdup(const char *s)
^ permalink raw reply related
* Re: [PATCH] pack-objects: don't use too many threads with few objects
From: Jeff King @ 2008-12-14 2:20 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LFD.2.00.0812131456040.30035@xanadu.home>
On Sat, Dec 13, 2008 at 03:06:40PM -0500, Nicolas Pitre wrote:
> If there are few objects to deltify, they might be split amongst threads
> so that there is simply no other objects left to delta against within
> the same thread. Let's use the same 2*window treshold as used for the
> final load balancing to allow extra threads to be created.
>
> This fixes the benign t5300 test failure.
I can confirm this fixes my t5300 failure. Thanks.
Tested-by: Jeff King <peff@peff.net>
-Peff
^ permalink raw reply
* Re: is gitosis secure?
From: Sitaram Chamarty @ 2008-12-14 2:26 UTC (permalink / raw)
To: git
In-Reply-To: <87hc58hwmi.fsf@hades.wkstn.nix>
On 2008-12-13, Nix <nix@esperi.org.uk> wrote:
> telnet. I do not jest, this is our sysadmins' stated reasons for not
> opening the git port and for tweaking their (mandatory) HTTP proxy to
> block HTTP traffic from git.
Wow -- my sympathies!
But on occasion, when real or imaginary issues prevented me
from making a live connection, I have used "git bundle" to
do the job. Not as satisfactory as a real connection, but
when you have a proper, non-fast-forwarding, repo as the
"mother ship", git bundle with some custom procmail scripts
on both sides can work OK enough.
To do that with a public repo you'd have to mirror that on a
home machine and let your restricted environment work
against that.
> Do not underestimate the stupidity and hideboundedness of undertrained
> system administrators, for it is vast.
These same administrators also underestimate (i) the number
of well connected home machines and (ii) the idea that on
his own machine, everyone is root.
Most of these blocks are "default allow", and your home IP
is not on that list and they don't have the smarts to figure
out that you're getting around their blocks :-) Add dynamic
IP and a dyndns hostname (and dyndns has a hundred or so 2nd
level domains to choose your 3rd level hostname from!) and
clueless admins don't stand a chance.
[sorry this is so badly off-topic...]
^ permalink raw reply
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