threads / patch / 4857

patchgitweb.cgi: Teach tree->raw to not require the hash of the blob

Subject: [PATCH] gitweb.cgi: Teach tree->raw to not require the hash of the blob

## tl;dr

7 messages between Jul 12, 2006 and Jul 14, 2006. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Luben Tuikov· Jul 12, 2006, 03:43 UTC · lore

Teach tree->raw to not require the hash of the blob, but to figure it out from the file name. This allows to externally link to files into the repository, such that the hash is not required. I.e. the file obtained would be as of the HEAD commit.

In contrast tree->blob for binary files passes the hash, as does tree->blob->plain for "text/*" files.

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
 gitweb/gitweb.cgi |   20 ++++++++++++++++----
 1 files changed, 16 insertions(+), 4 deletions(-)
Show changes to gitweb/gitweb.cgi +16 −4
diff --git a/gitweb/gitweb.cgi b/gitweb/gitweb.cgi
index 2e87de4..e904eb7 100755
--- a/gitweb/gitweb.cgi
+++ b/gitweb/gitweb.cgi
@@ -1521,6 +1521,14 @@ sub git_blob_plain_mimetype {
 }
 
 sub git_blob_plain {
+	if (!defined $hash) {
+		if (defined $file_name) {
+			my $base = $hash_base || git_read_head($project);
+			$hash = git_get_hash_by_path($base, $file_name, "blob") || die_error(undef, "Error lookup file.");
+		} else {
+			die_error(undef, "No file name defined.");
+		}
+	}
 	my $type = shift;
 	open my $fd, "-|", "$gitbin/git-cat-file blob $hash" or die_error("Couldn't cat $file_name, $hash");
 
@@ -1544,9 +1552,13 @@ sub git_blob_plain {
 }
 
 sub git_blob {
-	if (!defined $hash && defined $file_name) {
-		my $base = $hash_base || git_read_head($project);
-		$hash = git_get_hash_by_path($base, $file_name, "blob") || die_error(undef, "Error lookup file.");
+	if (!defined $hash) {
+		if (defined $file_name) {
+			my $base = $hash_base || git_read_head($project);
+			$hash = git_get_hash_by_path($base, $file_name, "blob") || die_error(undef, "Error lookup file.");
+		} else {
+			die_error(undef, "No file name defined.");
+		}
 	}
 	my $have_blame = git_get_project_config_bool ('blame');
 	open my $fd, "-|", "$gitbin/git-cat-file blob $hash" or die_error(undef, "Open failed.");
@@ -1678,8 +1690,7 @@ sub git_tree {
 			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$t_hash$base_key;f=$base$t_name")}, "blob") .
 #			      " | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blame;h=$t_hash$base_key;f=$base$t_name")}, "blame") .
 			      " | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=history;h=$hash_base;f=$base$t_name")}, "history") .
-			      " | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob_plain;h=$t_hash;f=$base$t_name")}, "raw") .
+			      " | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob_plain;f=$base$t_name")}, "raw") .
 			      "</td>\n";
 		} elsif ($t_type eq "tree") {
 			print "<td class=\"list\">" .

-- 
1.4.1.g9ca3
Junio C Hamano· Jul 12, 2006, 06:16 UTC · re: Luben Tuikov · lore

Re: [PATCH] gitweb.cgi: Teach tree->raw to not require the hash of the blob

Luben Tuikov <ltuikov@yahoo.com> writes:
Show 13 quoted lines
> Teach tree->raw to not require the hash of the blob, but to
> figure it out from the file name.  This allows to externally
> link to files into the repository, such that the hash is not
> required.  I.e. the file obtained would be as of the HEAD
> commit.
>
> In contrast tree->blob for binary files passes the hash, as
> does tree->blob->plain for "text/*" files.
>
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> ---
>  gitweb/gitweb.cgi |   20 ++++++++++++++++----
>  1 files changed, 16 insertions(+), 4 deletions(-)
This has exactly the same line number problem.
Show changes to diff +1 −1
@@ -1678,8 +1690,7 @@ sub git_tree {
 			      $cgi->a({-href => "$my_uri?" . esc...
 #			      " | " . $cgi->a({-href => "$my_uri...
 			      " | " . $cgi->a({-href => "$my_uri...
-			      " | " . $cgi->a({-href => "$my_uri...
+			      " | " . $cgi->a({-href => "$my_uri...
 			      "</td>\n";
 		} elsif ($t_type eq "tree") {
 			print "<td class=\"list\">" .

You are removing one line and adding one line, so the number of
old and new lines better match.

Hand-applied, tried, got confused and dropped.

I think _allowing_ to accept filename not hash is a sane change,
and would be useful if you want to allow linking to always the
HEAD version from external sites, but I do not think listing the
raw link in the tree view without the hash is a good idea.  It
makes things quite confusing that "blob" link in its
neighbourhood gives the blob from that specific version, but
"raw" gives the version from HEAD, even when you are browsing
something other than HEAD.

BTW, can somebody volunteer to be a gitweb/ "subsystem
maintainer"?
Jakub Narebski· Jul 12, 2006, 08:32 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitweb.cgi: Teach tree->raw to not require the hash of the blob

Junio C Hamano wrote:
Show 10 quoted lines
> Luben Tuikov <ltuikov@yahoo.com> writes:
> 
>> Teach tree->raw to not require the hash of the blob, but to
>> figure it out from the file name.  This allows to externally
>> link to files into the repository, such that the hash is not
>> required.  I.e. the file obtained would be as of the HEAD
>> commit.
>>
>> In contrast tree->blob for binary files passes the hash, as
>> does tree->blob->plain for "text/*" files.
Show 11 quoted lines
> I think _allowing_ to accept filename not hash is a sane change,
> and would be useful if you want to allow linking to always the
> HEAD version from external sites, but I do not think listing the
> raw link in the tree view without the hash is a good idea.  It
> makes things quite confusing that "blob" link in its
> neighbourhood gives the blob from that specific version, but
> "raw" gives the version from HEAD, even when you are browsing
> something other than HEAD.
> 
> BTW, can somebody volunteer to be a gitweb/ "subsystem
> maintainer"?

I think the change to raw aka. blob_plain and not to plain is because in plain view you have this HEAD link which takes us to HEAD revision of the file (even if we are on different branch IIRC).

Workaround for either format is to remove 'h' (hash) parameter entirely, and put in 'hb' (hashbase aka headref aka branch) the branch name. Gitweb then shows the contents of latest version of the file.

BTW. sometimes you want to link to specific version of the file, and sometimes to newest. I think it would be best (if possible, feasible, and not too much resource consuming) that links from tree specified by hash be to specified by hash version of file, while links from tree specified by headref and name only would have links to latest version of the file.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Luben Tuikov· Jul 12, 2006, 18:02 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb.cgi: Teach tree->raw to not require the hash of the blob

--- Jakub Narebski <jnareb@gmail.com> wrote:
Show 38 quoted lines
> Junio C Hamano wrote:
> 
> > Luben Tuikov <ltuikov@yahoo.com> writes:
> > 
> >> Teach tree->raw to not require the hash of the blob, but to
> >> figure it out from the file name.  This allows to externally
> >> link to files into the repository, such that the hash is not
> >> required.  I.e. the file obtained would be as of the HEAD
> >> commit.
> >>
> >> In contrast tree->blob for binary files passes the hash, as
> >> does tree->blob->plain for "text/*" files.
> 
> > I think _allowing_ to accept filename not hash is a sane change,
> > and would be useful if you want to allow linking to always the
> > HEAD version from external sites, but I do not think listing the
> > raw link in the tree view without the hash is a good idea.  It
> > makes things quite confusing that "blob" link in its
> > neighbourhood gives the blob from that specific version, but
> > "raw" gives the version from HEAD, even when you are browsing
> > something other than HEAD.
> > 
> > BTW, can somebody volunteer to be a gitweb/ "subsystem
> > maintainer"?
> 
> I think the change to raw aka. blob_plain and not to plain is because in
> plain view you have this HEAD link which takes us to HEAD revision of the
> file (even if we are on different branch IIRC).
> 
> Workaround for either format is to remove 'h' (hash) parameter entirely, and  
> put in 'hb' (hashbase aka headref aka branch) the branch name. Gitweb then
> shows the contents of latest version of the file.
> 
> BTW. sometimes you want to link to specific version of the file, and
> sometimes to newest. I think it would be best (if possible, feasible, and
> not too much resource consuming) that links from tree specified by hash be
> to specified by hash version of file, while links from tree specified by
> headref and name only would have links to latest version of the file.

You seem to have hand-edited the mail headers to remove Junio and myself from the CC/To headers list, but leave git@vger.

Please don't do that. Just click Reply-All and continue with your message.

Thanks,
    Luben
P.S. Restored.
Jakub Narebski· Jul 12, 2006, 18:12 UTC · re: Luben Tuikov · lore

Re: [PATCH] gitweb.cgi: Teach tree->raw to not require the hash of the blob

On 7/12/06, Luben Tuikov <ltuikov@yahoo.com> wrote:
Show 5 quoted lines
> You seem to have hand-edited the mail headers to remove
> Junio and myself from the CC/To headers list, but leave git@vger.
>
> Please don't do that.  Just click Reply-All and continue with your
> message.

No, I've just replied from newsreader (not mail client) via GMane news interface to git mailing list (gmane.comp.version-control.git), and I forgot to use Reply-All, i.e. use mail as well as news interface.

Besides I seem to have problems trying to configure KNode to send mail, especially with mixed mail+news sending.

-- 
Jakub Narebski
Luben Tuikov· Jul 12, 2006, 17:52 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitweb.cgi: Teach tree->raw to not require the hash of the blob

--- Junio C Hamano <junkio@cox.net> wrote:
Show 17 quoted lines
> Luben Tuikov <ltuikov@yahoo.com> writes:
> 
> > Teach tree->raw to not require the hash of the blob, but to
> > figure it out from the file name.  This allows to externally
> > link to files into the repository, such that the hash is not
> > required.  I.e. the file obtained would be as of the HEAD
> > commit.
> >
> > In contrast tree->blob for binary files passes the hash, as
> > does tree->blob->plain for "text/*" files.
> >
> > Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> > ---
> >  gitweb/gitweb.cgi |   20 ++++++++++++++++----
> >  1 files changed, 16 insertions(+), 4 deletions(-)
> 
> This has exactly the same line number problem.
[cut]
> Hand-applied, tried, got confused and dropped.
I'll regenerate and send it to the list.
> I think _allowing_ to accept filename not hash is a sane change,
> and would be useful if you want to allow linking to always the
> HEAD version from external sites,
Indeed, it is useful.
Show 6 quoted lines
> but I do not think listing the
> raw link in the tree view without the hash is a good idea.  It
> makes things quite confusing that "blob" link in its
> neighbourhood gives the blob from that specific version, but
> "raw" gives the version from HEAD, even when you are browsing
> something other than HEAD.

I just thought it to be an easy place to put the "raw"-no-hash link.

BTW, Junio, it would be a shame to lose this capability. How would you like to proceed with this? Where would you like to see this kind of link go?

> BTW, can somebody volunteer to be a gitweb/ "subsystem
> maintainer"?

Make sure you pick someone liberal without personal aspirations or turning it into a personal crusade. Service to the community (not self, or company XYZ) is best.

    Luben
Junio C Hamano· Jul 14, 2006, 05:49 UTC · re: Luben Tuikov · lore

Re: [PATCH] gitweb.cgi: Teach tree->raw to not require the hash of the blob

Luben Tuikov <ltuikov@yahoo.com> writes:
Show 19 quoted lines
>> I think _allowing_ to accept filename not hash is a sane change,
>> and would be useful if you want to allow linking to always the
>> HEAD version from external sites,
>
> Indeed, it is useful.
>
>> but I do not think listing the
>> raw link in the tree view without the hash is a good idea.  It
>> makes things quite confusing that "blob" link in its
>> neighbourhood gives the blob from that specific version, but
>> "raw" gives the version from HEAD, even when you are browsing
>> something other than HEAD.
>
> I just thought it to be an easy place to put the "raw"-no-hash
> link.
>
> BTW, Junio, it would be a shame to lose this capability.  How
> would you like to proceed with this?  Where would you like to
> see this kind of link go?
My preference?

Allowing to link the HEAD version from an _external_ source is useful (i.e. you can put a link in gitwiki to point at a file and say "the latest is always available at this URL").

We already support these:
        a=blob&f=README
        a=blob&f=README&hb=HEAD
        a=blob&f=README&hb=HEAD~20
but not these:
        a=blob_plain&f=README
        a=blob_plain&f=t/test4012.png

The last example that does not generate text is less of a problem, thanks to your previous patch, because "sub git_blob" supports the "filename with or without hash base" syntax and sends the correct hash to git_blob_plain to fall back, but that does not help for text files. This patch corrects that, which is nice.

However, I do not think the change to "sub git_tree" makes much sense. The links within gitweb-generated pages are about browsing the history. As I already said, having <blob> link that is history specific and <raw> link which is not (iow always goes to HEAD) next to each other is confusing, and you are making it less easy to get a raw output from a specific version because you removed that feature from the link and replaced it with something less useful.

If somebody wants to see what the latest version of a blob looks like while browsing the history with gitweb, I think it is a more natural workflow to go to <history> link and find the latest version. So maybe you might want to add <raw> link in the output of the history action?

        Side note: right now, <history> link does not show any
        history newer than the commit that has the link itself;
        maybe there is a room for improvement there but we need
        to be careful not to trash the webserver caching.

Obviously you could go back to the repository top page and start digging the latest tree from the top, but that is a bit more cumbersome.

← back to recent threads