New issue
Advanced search Search tips
Starred by 40 users
Status: Released
Owner: ----
Closed: Aug 2015



Sign in to add a comment
Repository viewer
Reported by mike.lif...@gmail.com, Mar 13 2010 Back to list
Gerrit should include a repository viewer to browse through the source
tree. This should be subject to the same access restrictions as viewing
changesets etc.
 
Comment 1 by sop@google.com, Mar 13 2010
Status: AwaitingInformation
Is the gitweb integration not sufficient?
Not really. Code review invariably involves whole-file review. If Gerrit is a code
review tool, it should allow proper whole-file review, which means a repository viewer.

However, on practical grounds, gitweb has no authentication of it's own. While it may
be possible, depending on setup, to use http auth, this is not desirable because it
involves extra setup, and doesn't use Gerrit's own access restrictions.

Note this is almost possible because Gerrit already allows viewing whole files in the
side-by-side diff for changesets. However, this is insufficient because it doesn't
allow (for example) viewing a file that didn't change in a particular changeset, or
viewing the whole tree at a particular commit.
Comment 3 by sop@google.com, Mar 13 2010
Oh heck.  gitweb isn't fully documented, even in 2.1.2's docs.
We have a CGI servlet which knows how to filter parameters for
gitweb, and does access control checks before running it [1].

But I've also wanted a way to add any other file into a review,
even though it wasn't modified by the user, so you can examine
it.  And if you add a comment, then it would show up as part of
the review, but with a blank status or something else to indicate
it wasn't modified, just commented on.  This actually isn't hard,
it just would take a bit of UI work to implement the file search
inside of that file list popup dialog, or on the top level of the
change page.

[1] http://android.git.kernel.org/?p=tools/gerrit.git;a=tree;f=gerrit-
httpd/src/main/java/com/google/gerrit/httpd/gitweb;h=ec0eaff8c6f75b1b132f5817d5558996
fac52337;hb=60cd31bce7f7846c156becf321e945eb08c65f45
Comment 4 by sop@google.com, Mar 13 2010
Tangentially related is JGit bug 299886 [2], which asks for
a servlet to provide a web based repository browser.  I'm
not sure if the JGit project would accept something based
only on a GWT/AJAX UI.

Ideally, if we actually add a full viewer, the effort gets put
into JGit's feature and we can just reuse it here in Gerrit.

[2] https://bugs.eclipse.org/bugs/show_bug.cgi?id=299886
So, this GCI servlet will provide a gitweb interface, but with Gerrit's access
controls? That's something you'll want to document(!) - and as soon as you do, I'll
give it a shot.
Comment 6 by sop@google.com, Mar 13 2010
Status: Started
Documented [3].

[3] http://gerrit.googlecode.com/svn/documentation/2.1.2/config-gitweb.html
Where does the configuration for gitweb come from? It doesn't look like it is using
the main file - or at least some of is it being overwritten? This means I can't fix
the following issues:
*trying to load /gerrit/gitweb-default.css, but there is no such file.
*same for logo
*well, I can't change any settings at all if I can't find the config file it is using
Some apache tweaks can get the first two working.

Also, links point to /gitweb.cgi... (which is okish, I do have gitweb accessible
there), but I guess there must be some way for them to point to /gerrit/gitweb.cgi?

And yes, this will cover what I need - I'd resolve this as WORKSFORME but I don't see
how.
Comment 8 by sop@google.com, Mar 13 2010
Status: AwaitingInformation
The configuration file is generated by Gerrit and written
out to a temporary file.  Its in your system temporary
directory as gitweb_config_*.perl.  That may actually be
a bug, some systems have /tmp cleaners that automatically
remove files older than a week.

The CGI is mounted as /gitweb within the Gerrit context.
CSS and logo are /gitweb-default.css and /gitweb-logo.png.
So these should all be accessible and served out of your
Gerrit context.  If they aren't, its a bug.

Links should be going to /gitweb not /gitweb.cgi within
the Gerrit context.  So really on a page they should be
"gitweb?..." as they are relative to the page displaying
the web interface.

Did you ensure gitweb.url is *NOT* set in your gerrit.config?
CSS & logo aren't served up by the Jetty server. I made exemptions for them with
ProxyPass and RedirectMatch so apache would grab them from my "real" gitweb install.
I'll open a new bug for that shortly.

Links on the page are going outside the Jetty version of gitweb & I don't see how to
change that. I'll try playing with the temp config file, but unless there's a way to
have persistent configuration for this, it won't be much use.
Comment 10 by sop@google.com, Apr 24 2010
Status: Accepted
2.1.2.1 should have fixed most of the issues with
gitweb serving resources... and it documented how
to configure it.

This issue is still open however because just using
gitweb isn't really sufficient.  We have to disable
it in the presence of branch level access controls,
but we really should still support a viewer even in
that configuration.
This is my biggest gripe with Gerrit. I seriously miss an integrated proper code browse tool, like e.g. "Grepcode"* with both a tree and "go to definition"-style links etc., and possibly "link out" to other projects, both internal, and to external tools like Grepcode to understand any libraries that are being called.

If this was included as a part of Gerrit and the differ, it would make the reviewing process much better: Both more pleasant due to ability to understand the code better with better navigation and context, but also qualitatively better in that better availability of context and ease of understanding of the change in its setting would make the actual review quality better.

* http://grepcode.com/file/repo1.maven.org/maven2/org.eclipse.jetty/jetty-server/9.2.2.v20140723/org/eclipse/jetty/server/Server.java?av=f
Project Member Comment 12 by nas...@grainawi.org, Aug 22 2015
Status: Closed
I think Gitiles serves this purpose fairly well now (especially as a plugin with full support for Gerrit ACLs).
Project Member Comment 13 by nas...@grainawi.org, Aug 22 2015
Status: Released
Sign in to add a comment