[RFC] gitweb: add 'historyfollow' view that follows renames

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

[RFC] gitweb: add 'historyfollow' view that follows renames

From: Blucher, Guy <hidden>
Date: 2016-06-15 22:45:32

Hi Folks,
quoted
What should we add to automatically get all file history?
While the 'commitdiff' view would, in default gitweb configuration, 
contain information about file renames, currently 'history' view does 
not support '--follow' option to git-log.  It wouldn't be too hard to 
add it, but it just wasn't done (well, add to this the fact that 
--follow works only for simple cases).
We also ran up against this issue because renaming of files in our
project is a useful bit of information while browsing file history.

I hacked gitweb to add a 'historyfollow' viewing option in addition to
the 'history' option.  Yes, '--follow' is expensive so I didn't just
make it the default 'history' behaviour. 

The only problem with doing it was that parse_commits in gitweb.perl
uses git rev-list which doesn't support the '--follow' option like
git-log does. A bit of hacking to use 'git log --pretty=raw -z' to make
the output look close to that from rev-list means only minor
shoe-horning is required (perhaps it would be better to make rev-list
understand --follow though?).

I wasn't originally prepared to publish the work here because I don't
really think it's the best solution. But considering it's come up... I
include a patch inline against gitweb.perl from v1.6.0.3 that implements
a 'historyfollow' view. 

Feel free to do with it what you will. It would need some substantial
tidying up by someone who knows much more about perl than me :) 

We use it such that anywhere a 'history' link is provided a
'historyfollow' link is also provided next to it - This patch doesn't
implement that bit though.

Hope it helps.

Cheers,
Guy.

---
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -478,6 +478,7 @@ my %actions = (
        "forks" => \&git_forks,
        "heads" => \&git_heads,
        "history" => \&git_history,
+       "historyfollow" => \&git_history_follow,
        "log" => \&git_log,
        "rss" => \&git_rss,
        "atom" => \&git_atom,
@@ -2311,25 +2312,39 @@ sub parse_commit {
 }
 
 sub parse_commits {
-       my ($commit_id, $maxcount, $skip, $filename, @args) = @_;
+       my ($commit_id, $maxcount, $skip, $mode, $filename, @args) = @_;
        my @cos;
 
        $maxcount ||= 1;
        $skip ||= 0;
 
        local $/ = "\0";
+        # The '--max-count' argument is not available when doing a
+        # '--follow' to 'git log'
+        my $count_arg = ("--max-count=" . $maxcount) ;
+        if (defined $mode && $mode eq "--follow") {
+            $count_arg = "--follow" ;
+        }
 
-       open my $fd, "-|", git_cmd(), "rev-list",
-               "--header",
+
+       open my $fd, "-|", git_cmd(), "log",
+               "-z",
+               "--pretty=raw",
                @args,
-               ("--max-count=" . $maxcount),
+                ($count_arg ? ($count_arg ) : ()),
                ("--skip=" . $skip),
                @extra_options,
                $commit_id,
                "--",
                ($filename ? ($filename) : ())
-               or die_error(500, "Open git-rev-list failed");
+               or die_error(500, "Open git-log failed");
        while (my $line = <$fd>) {
+               # Need to put a delimiter on the end of output
+                # 'git-log -z' doesn't put one before EOF like rev-list
does
+                $line = ($line . '\0');
+                # Need to strip the word commit from the start so it
+                # looks like rev-list output
+                $line =~ s/^commit // ;
                my %co = parse_commit_text($line);
                push @cos, \%co;
        }
@@ -5363,6 +5378,13 @@ sub git_commitdiff_plain {
 }
 
 sub git_history {
+        my $mode = shift || '';
+        my $history_call = "history";
+
+       if ($mode eq "--follow") {
+           $history_call .= "historyfollow" ;
+       }
+
        if (!defined $hash_base) {
                $hash_base = git_get_head_hash($project);
        }
@@ -5377,7 +5399,7 @@ sub git_history {
        my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
 
        my @commitlist = parse_commits($hash_base, 101, (100 * $page),
-                                      $file_name, "--full-history")
+                                      $mode, $file_name,
"--full-history")
            or die_error(404, "No such file or directory on given
branch");
 
        if (!defined $hash && defined $file_name) {
@@ -5398,7 +5420,7 @@ sub git_history {
        my $paging_nav = '';
        if ($page > 0) {
                $paging_nav .=
-                       $cgi->a({-href => href(action=>"history",
hash=>$hash, hash_base=>$hash_base,
+                       $cgi->a({-href => href(action=>"$history_call",
hash=>$hash, hash_base=>$hash_base,
                                               file_name=>$file_name)},
                                "first");
                $paging_nav .= " &sdot; " .
@@ -5429,6 +5451,11 @@ sub git_history {
        git_footer_html();
 }
 
+sub git_history_follow {
+       git_history('--follow');
+}
+
+
 sub git_search {
        gitweb_check_feature('search') or die_error(403, "Search is
disabled");
        if (!defined $searchtext) {
@@ -5469,7 +5496,7 @@ sub git_search {
                        $greptype = "--committer=";
                }
                $greptype .= $searchtext;
-               my @commitlist = parse_commits($hash, 101, (100 *
$page), undef,
+               my @commitlist = parse_commits($hash, 101, (100 *
$page), undef, undef,
                                               $greptype,
'--regexp-ignore-case',
                                               $search_use_regexp ?
'--extended-regexp' : '--fixed-strings');
 
@@ -5737,7 +5764,7 @@ sub git_feed {
 
        # log/feed of current (HEAD) branch, log of given branch,
history of file/directory
        my $head = $hash || 'HEAD';
-       my @commitlist = parse_commits($head, 150, 0, $file_name);
+       my @commitlist = parse_commits($head, 150, 0, undef,
$file_name);
 
        my %latest_commit;
        my %latest_date;
---

Guy.
____________________________________________________
Guy Blucher
Defence Science and Technology Organisation
AUSTRALIA

IMPORTANT : This email remains the property of the Australian Defence
Organisation and is subject to the jurisdiction of section 70 of the
Crimes Act 1914.  If you have received this email in error, you are
requested to contact the sender and delete the email.

RE: [RFC] gitweb: add 'historyfollow' view that follows renames

From: Moore, Robert <hidden>
Date: 2016-06-15 22:45:33

Thanks. This sounds like what we need.

We will take a look at it.
Bob

quoted hunk
-----Original Message-----
From: Blucher, Guy [mailto:Guy.Blucher@dsto.defence.gov.au]
Sent: Sunday, October 26, 2008 8:18 PM
To: Lin, Ming M; jnareb@gmail.com; Moore, Robert
Cc: git@vger.kernel.org
Subject: [RFC] gitweb: add 'historyfollow' view that follows renames


Hi Folks,
quoted
quoted
What should we add to automatically get all file history?
quoted
While the 'commitdiff' view would, in default gitweb configuration,
contain information about file renames, currently 'history' view does
not support '--follow' option to git-log.  It wouldn't be too hard to
add it, but it just wasn't done (well, add to this the fact that
--follow works only for simple cases).
We also ran up against this issue because renaming of files in our
project is a useful bit of information while browsing file history.

I hacked gitweb to add a 'historyfollow' viewing option in addition to
the 'history' option.  Yes, '--follow' is expensive so I didn't just
make it the default 'history' behaviour.

The only problem with doing it was that parse_commits in gitweb.perl
uses git rev-list which doesn't support the '--follow' option like
git-log does. A bit of hacking to use 'git log --pretty=raw -z' to make
the output look close to that from rev-list means only minor
shoe-horning is required (perhaps it would be better to make rev-list
understand --follow though?).

I wasn't originally prepared to publish the work here because I don't
really think it's the best solution. But considering it's come up... I
include a patch inline against gitweb.perl from v1.6.0.3 that implements
a 'historyfollow' view.

Feel free to do with it what you will. It would need some substantial
tidying up by someone who knows much more about perl than me :)

We use it such that anywhere a 'history' link is provided a
'historyfollow' link is also provided next to it - This patch doesn't
implement that bit though.

Hope it helps.

Cheers,
Guy.

---
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -478,6 +478,7 @@ my %actions = (
       "forks" => \&git_forks,
       "heads" => \&git_heads,
       "history" => \&git_history,
+       "historyfollow" => \&git_history_follow,
       "log" => \&git_log,
       "rss" => \&git_rss,
       "atom" => \&git_atom,
@@ -2311,25 +2312,39 @@ sub parse_commit {
}

sub parse_commits {
-       my ($commit_id, $maxcount, $skip, $filename, @args) = @_;
+       my ($commit_id, $maxcount, $skip, $mode, $filename, @args) = @_;
       my @cos;

       $maxcount ||= 1;
       $skip ||= 0;

       local $/ = "\0";
+        # The '--max-count' argument is not available when doing a
+        # '--follow' to 'git log'
+        my $count_arg = ("--max-count=" . $maxcount) ;
+        if (defined $mode && $mode eq "--follow") {
+            $count_arg = "--follow" ;
+        }

-       open my $fd, "-|", git_cmd(), "rev-list",
-               "--header",
+
+       open my $fd, "-|", git_cmd(), "log",
+               "-z",
+               "--pretty=raw",
               @args,
-               ("--max-count=" . $maxcount),
+                ($count_arg ? ($count_arg ) : ()),
               ("--skip=" . $skip),
               @extra_options,
               $commit_id,
               "--",
               ($filename ? ($filename) : ())
-               or die_error(500, "Open git-rev-list failed");
+               or die_error(500, "Open git-log failed");
       while (my $line = <$fd>) {
+               # Need to put a delimiter on the end of output
+                # 'git-log -z' doesn't put one before EOF like rev-list
does
+                $line = ($line . '\0');
+                # Need to strip the word commit from the start so it
+                # looks like rev-list output
+                $line =~ s/^commit // ;
               my %co = parse_commit_text($line);
               push @cos, \%co;
       }
@@ -5363,6 +5378,13 @@ sub git_commitdiff_plain {
}

sub git_history {
+        my $mode = shift || '';
+        my $history_call = "history";
+
+       if ($mode eq "--follow") {
+           $history_call .= "historyfollow" ;
+       }
+
       if (!defined $hash_base) {
               $hash_base = git_get_head_hash($project);
       }
@@ -5377,7 +5399,7 @@ sub git_history {
       my $limit = sprintf("--max-count=%i", (100 * ($page+1)));

       my @commitlist = parse_commits($hash_base, 101, (100 * $page),
-                                      $file_name, "--full-history")
+                                      $mode, $file_name,
"--full-history")
           or die_error(404, "No such file or directory on given
branch");

       if (!defined $hash && defined $file_name) {
@@ -5398,7 +5420,7 @@ sub git_history {
       my $paging_nav = '';
       if ($page > 0) {
               $paging_nav .=
-                       $cgi->a({-href => href(action=>"history",
hash=>$hash, hash_base=>$hash_base,
+                       $cgi->a({-href => href(action=>"$history_call",
hash=>$hash, hash_base=>$hash_base,
                                              file_name=>$file_name)},
                               "first");
               $paging_nav .= " &sdot; " .
@@ -5429,6 +5451,11 @@ sub git_history {
       git_footer_html();
}

+sub git_history_follow {
+       git_history('--follow');
+}
+
+
sub git_search {
       gitweb_check_feature('search') or die_error(403, "Search is
disabled");
       if (!defined $searchtext) {
@@ -5469,7 +5496,7 @@ sub git_search {
                       $greptype = "--committer=";
               }
               $greptype .= $searchtext;
-               my @commitlist = parse_commits($hash, 101, (100 *
$page), undef,
+               my @commitlist = parse_commits($hash, 101, (100 *
$page), undef, undef,
                                              $greptype,
'--regexp-ignore-case',
                                              $search_use_regexp ?
'--extended-regexp' : '--fixed-strings');
@@ -5737,7 +5764,7 @@ sub git_feed {

       # log/feed of current (HEAD) branch, log of given branch,
history of file/directory
       my $head = $hash || 'HEAD';
-       my @commitlist = parse_commits($head, 150, 0, $file_name);
+       my @commitlist = parse_commits($head, 150, 0, undef,
$file_name);

       my %latest_commit;
       my %latest_date;
---

Guy.
____________________________________________________
Guy Blucher
Defence Science and Technology Organisation
AUSTRALIA

IMPORTANT : This email remains the property of the Australian Defence
Organisation and is subject to the jurisdiction of section 70 of the
Crimes Act 1914.  If you have received this email in error, you are
requested to contact the sender and delete the email.

Re: [RFC] gitweb: add 'historyfollow' view that follows renames

From: Jakub Narebski <hidden>
Date: 2016-06-15 22:45:33

I'm sorry for the delay in reviewing this patch...

On Mon, 27 Oct 2008, Huy Blucher wrote:

[Please try do not lose attributions...]
quoted
quoted
What should we add to automatically get all file history?
quoted
While the 'commitdiff' view would, in default gitweb configuration, 
contain information about file renames, currently 'history' view does 
not support '--follow' option to git-log.  It wouldn't be too hard to 
add it, but it just wasn't done (well, add to this the fact that 
--follow works only for simple cases).
We also ran up against this issue because renaming of files in our
project is a useful bit of information while browsing file history.

I hacked gitweb to add a 'historyfollow' viewing option in addition to
the 'history' option.  Yes, '--follow' is expensive so I didn't just
make it the default 'history' behaviour.
I would prefer if instead of adding new 'historyfollow' action, which
goes against stated in TODO goal of uniquifying log-like views handling,
the patch added support for '--follow' as extra option to 'history'
view (i.e. a=history;opt=--follow)... on the other hand 'historyfollow'
(or simply 'follow') can be used in pure path_info gitweb URL... Hmm...
The only problem with doing it was that parse_commits in gitweb.perl
uses git rev-list which doesn't support the '--follow' option like
git-log does. A bit of hacking to use 'git log --pretty=raw -z' to make
the output look close to that from rev-list means only minor
shoe-horning is required (perhaps it would be better to make rev-list
understand --follow though?).
Either that, or use --pretty=format:<sth> which mimics current use of
"git-rev-list <opts>" output in parse_commits exactly.

And either move parse_commits to use git-log, change git-rev-parse to
understand '--follow', or make parse_commits use git-log with --follow,
git-rev-list otherwise.
I wasn't originally prepared to publish the work here because I don't
really think it's the best solution. But considering it's come up... I
include a patch inline against gitweb.perl from v1.6.0.3 that implements
a 'historyfollow' view.
RFC (usually marked [RFC/PATCH]) patches are good because they allow
others to test and comment on your solution: early review, better to
spot bugs etc. earlier.
Feel free to do with it what you will. It would need some substantial
tidying up by someone who knows much more about perl than me :) 

We use it such that anywhere a 'history' link is provided a
'historyfollow' link is also provided next to it - This patch doesn't
implement that bit though.

Hope it helps.

Cheers,
Guy.
It would be nice though if such [RFC/PATCH] followed guidelines from
SubmittingPatches on commit messages...
---
Diffstat?
quoted hunk
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -478,6 +478,7 @@ my %actions = (
        "forks" => \&git_forks,
        "heads" => \&git_heads,
        "history" => \&git_history,
+       "historyfollow" => \&git_history_follow,
        "log" => \&git_log,
        "rss" => \&git_rss,
        "atom" => \&git_atom,
@@ -2311,25 +2312,39 @@ sub parse_commit {
 }
 
 sub parse_commits {
-       my ($commit_id, $maxcount, $skip, $filename, @args) = @_;
+       my ($commit_id, $maxcount, $skip, $mode, $filename, @args) = @_;
Can't you simply pass '--follow' or 'follow' in @args instead of
changing the signature of parse_commits?
        my @cos;
 
        $maxcount ||= 1;
        $skip ||= 0;
 
        local $/ = "\0";
+        # The '--max-count' argument is not available when doing a
+        # '--follow' to 'git log'
+        my $count_arg = ("--max-count=" . $maxcount) ;
+        if (defined $mode && $mode eq "--follow") {
+            $count_arg = "--follow" ;
+        }
I don't understand. Do you mean that

  $ git log --max-count=<n> --follow <rev> -- <path>

doesn't work as expected? Hmmm... true, it doesn't work. 
Then it is certainly a _BUG_ in git!
 
-       open my $fd, "-|", git_cmd(), "rev-list",
-               "--header",
+
+       open my $fd, "-|", git_cmd(), "log",
+               "-z",
+               "--pretty=raw",
                @args,
-               ("--max-count=" . $maxcount),
+                ($count_arg ? ($count_arg ) : ()),
Whitespace damage (here visible).
                ("--skip=" . $skip),
                @extra_options,
                $commit_id,
                "--",
                ($filename ? ($filename) : ())
-               or die_error(500, "Open git-rev-list failed");
+               or die_error(500, "Open git-log failed");
        while (my $line = <$fd>) {
+               # Need to put a delimiter on the end of output
+                # 'git-log -z' doesn't put one before EOF like rev-list
does
+                $line = ($line . '\0');
Doesn't it work if you don't add the above line?
+                # Need to strip the word commit from the start so it
+                # looks like rev-list output
+                $line =~ s/^commit // ;
                my %co = parse_commit_text($line);
Perhaps we should update parse_commit_text instead... or add an option
like parse_commit_text($line, -format=>'log'), or something like that.
quoted hunk
                push @cos, \%co;
        }
@@ -5363,6 +5378,13 @@ sub git_commitdiff_plain {
 }
 
 sub git_history {
+        my $mode = shift || '';
+        my $history_call = "history";
+
+       if ($mode eq "--follow") {
+           $history_call .= "historyfollow" ;
+       }
+
I don't quite like this solution...

If '--follow' was passed through @extra_options ('opt') parameter, then
it should be re-used in links thanks to href(..., -replay=>1).
quoted hunk
        if (!defined $hash_base) {
                $hash_base = git_get_head_hash($project);
        }
@@ -5377,7 +5399,7 @@ sub git_history {
        my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
 
        my @commitlist = parse_commits($hash_base, 101, (100 * $page),
-                                      $file_name, "--full-history")
+                                      $mode, $file_name,
"--full-history")
Word wrapped (not only here).
quoted hunk
            or die_error(404, "No such file or directory on given
branch");
 
        if (!defined $hash && defined $file_name) {
@@ -5398,7 +5420,7 @@ sub git_history {
        my $paging_nav = '';
        if ($page > 0) {
                $paging_nav .=
-                       $cgi->a({-href => href(action=>"history",
hash=>$hash, hash_base=>$hash_base,
+                       $cgi->a({-href => href(action=>"$history_call",
hash=>$hash, hash_base=>$hash_base,
                                               file_name=>$file_name)},
                                "first");
I would add opts=>[@extra_options] instead of changing action=>"history"
to action=>$history_call (the quotes around variable are not needed).
quoted hunk
                $paging_nav .= " &sdot; " .
@@ -5429,6 +5451,11 @@ sub git_history {
        git_footer_html();
 }
 
+sub git_history_follow {
+       git_history('--follow');
+}
+
+
 sub git_search {
        gitweb_check_feature('search') or die_error(403, "Search is
disabled");
        if (!defined $searchtext) {
@@ -5469,7 +5496,7 @@ sub git_search {
                        $greptype = "--committer=";
                }
                $greptype .= $searchtext;
-               my @commitlist = parse_commits($hash, 101, (100 *
$page), undef,
+               my @commitlist = parse_commits($hash, 101, (100 *
$page), undef, undef,
                                               $greptype,
'--regexp-ignore-case',
                                               $search_use_regexp ?
'--extended-regexp' : '--fixed-strings');
 
@@ -5737,7 +5764,7 @@ sub git_feed {
 
        # log/feed of current (HEAD) branch, log of given branch,
history of file/directory
        my $head = $hash || 'HEAD';
-       my @commitlist = parse_commits($head, 150, 0, $file_name);
+       my @commitlist = parse_commits($head, 150, 0, undef,
$file_name);
 
        my %latest_commit;
        my %latest_date;
---
What I would like to see is the link in the bottom of action bar
(navbar) or just below it, which would list 'follow' as one of possible
'history' view formats (just like '--no-merges' or '--first-parent'
should be).

-- 
Jakub Narebski
Poland
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help