My favorites | Sign in
Project Home Downloads Wiki Issues Source
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 3330: Side-by-Side reviews are blank
5 people starred this issue and may be notified of changes. Back to list
Status:  AwaitingInformation
Owner:  ----


Sign in to add a comment
 
Reported by mikel...@aol.com, Apr 23, 2015
*****************************************************************
*****                                                       *****
***** !!!! THIS BUG TRACKER IS FOR GERRIT CODE REVIEW !!!!  *****
*****                                                       *****
***** DO NOT SUBMIT BUGS FOR CHROME, ANDROID, CYANOGENMOD,  *****
***** INTERNAL ISSUES WITH YOUR COMPANY'S GERRIT SETUP, ETC.*****
*****                                                       *****
*****   THOSE ISSUES BELONG IN DIFFERENT ISSUE TRACKERS     *****
*****                                                       *****
*****************************************************************
This bug should be a very high priority since this is the mainstay of what gerrit does. It helps you communicate and visualize the changes in code.

Affected Version:

What steps will reproduce the problem?
1. Create a change to a .m or .h file and push changes to HEAD:refs/for/master
2. Attempt to view the change in side-by-side diff in Gerrit. 
3. Side-by-side diff is shows blank.

What is the expected output?
Expected output is the diff in side-by-side format.
What do you see instead? Blank! With the label, "No Differences". This is misleading.
screenshot attached.

Workaround:
Unified Diff still works.

Please provide any additional information below.
Gerrit 2.11 Official Release.
Chrome and Safari.

Not sure if any of this is relevant, but it describes my system architecture.
Gerrit is fronted by Apache2. Using Reverse Proxy on ports 80/443 and gerrit.war listens on proxy-http= localhost:8081. 
Ubuntu 14.04
Using HTTP authentication with Single Sign-on. Apache is handling the SSO transaction and is setting a header and passing it to Gerrit. 

Screen Shot 2015-04-23 at 7.29.32 PM.png
130 KB   View   Download
Apr 28, 2015
#1 mikel...@aol.com
EDIT: Also, using postgresql UTF-8 encoded. 
Apr 28, 2015
#2 Testingk...@gmail.com
Saw the same bug, everything seems empty. We moved to Gerrit but this thing is blocked our project. This should be on high priority.
Apr 29, 2015
Project Member #3 edwin.ke...@gmail.com
I'm not able to reproduce this.

> Create a change to a .m or .h file
Can you confirm that this is only happening for .m and .h files?
Does it work for files with other extensions?

> Using Reverse Proxy
This might be relevant. Have a look at  issue 2503  and [1] and check whether your setting to allow encoded slashes is correct.

[1] https://gerrit-review.googlesource.com/Documentation/install-j2ee.html#tomcat

Apr 29, 2015
#4 mikel...@aol.com
Thank you for your response.
I am not running behind tomcat but am using reverse proxy behind apache2.
It's also happening with a simple text file. Not just Xcode stuff. 
There is one exception.
When I make changes to a project configuration then (and seemingly only this case) does side-by-side diff work on the project config diff.
In all cases the diff does NOT launch a new browser window which is the expected behavior. If it did I would suspect encoded slashes settings to be incorrectly set in my Apache2 config file. 

Apr 29, 2015
#5 Testingk...@gmail.com
It also happens with build.gradle and txt file with me
Apr 29, 2015
#6 Testingk...@gmail.com
In Settings > Preferences if I change diff view to : Unified it works fine but side by side is the problem
Apr 30, 2015
#7 mikel...@aol.com
Here is my scrubbed httpd config. Note, we use mod-rewrite. Gerrit is served NOT on /r/ but on /gerrit/
Also, AllowEncodedSlashes On causes the user to see "Not Found" when clicking on a file in a review. We have to use NoDecode.

<VirtualHost _default_:443>
	ServerName gerrit.sub.corp.example.com
	SSLEngine On

	RewriteEngine On
	
	RewriteCond %{HTTP_HOST} !=gerrit.sub.corp.example.com
	RewriteRule ^.*$ https://gerrit.sub.corp.example.com%{REQUEST_URI}
#Using NoDecode because when set to “On” it causes “not found” to appear when clicking on a file to be reviewed.
AllowEncodedSlashes NoDecode
	<Location />
		RewriteEngine on

		RewriteCond %{REQUEST_URI} !^/favicon.ico
		RewriteCond %{REQUEST_URI} !^/gerrit/?.*
		RewriteCond %{REQUEST_URI} !^/resources/?.*
		RewriteCond %{REQUEST_URI} !^/PubCookie\.reply$
		# Temporary home page
		RewriteRule ^.*$ https://gerrit.sub.corp.example.com/gerrit/
	</Location>

	# Access to anything under /gerrit/ is handled by tomcat which is running at 
	# localhost:8081.	Note that [P] implies [L], so stop here. 
	RewriteRule ^/gerrit/?(.*) http://localhost:8081/gerrit/$1 [P]
	
	# Outbound information from gerrit’s Jetty is rewritten to refer to /gerrit/ rather
	# than localhost:8081.
	ProxyPassReverse /gerrit/ http://localhost:8081/gerrit/

        #Handle the SSO login.
        <Location /gerrit/login/>
          # Ensure that the REMOTE-USER header is set to indicate the authenticated username
          #also ensure that httpHeader=REMOTE-USER is set in the gerrit config file. 
          RewriteRule .* - [E=PROXY_USER:%{LA-U:REMOTE_USER}] # note mod_rewrite's lookahead option
          RequestHeader set REMOTE-USER %{PROXY_USER}e
          SSLRequireSSL 
          AuthType ProprietaryNet
          AuthName "Gerrit Code Review"
          PubcookieInactiveExpire -1
          PubcookieAppID gerrit.sub.corp.example.com
          Require valid-user
        </Location>

</VirtualHost>

Here is my scrubbed Gerrit Config.
[gerrit]
	basePath = git
	canonicalWebUrl = http://gerrit.sub.corp.example.com/
[database]
	type = postgresql
	hostname = gerrit-db.corp.example.com
	port = 5432
	database = reviewdb
	username = gerrit2
[index]
	type = LUCENE
[auth]
	type = HTTP
	httpHeader = REMOTE-USER
        emailFormat = {0}@example.com
	logoutUrl = http://weblogin.example.com
[user]
	name = "Gerrit Code Review"
        email = noreply@example.com
[sendemail]
	smtpServer = mail-relay.example.com
        from = SERVER
[container]
	user = root
	javaHome = /usr/lib/jvm/java-7-openjdk-amd64/jre
[sshd]
	listenAddress = *:29418
[httpd]
	listenUrl = proxy-http://*:8081/gerrit/
[cache]
	directory = cache

[plugins]
       allowRemoteAdmin = true
May 6, 2015
#8 mikel...@aol.com
Unsure if this is a problem, but there are spaces in the paths to the files. 
May 11, 2015
#9 sgunder...@bigfoot.com
We see the same issue. Reverse proxy behind Apache 2.2, AllowEncodedSlashes NoDecode (since everything else gives 404 problems). No space in filenames. Unified diff, on the other hand, works fine.
May 14, 2015
#10 zaharo...@gmail.com
Just upgraded 2.9 to 2.11. Unified view works fine, but side-by-side is empty.
May 20, 2015
#11 tomiphon...@gmail.com
I'm in 2.11 and now it works for me with this apache configuration : 

<VirtualHost 192.168.102.210:80>

    ServerName        gerrit
    ServerAlias        gerrit.uem.lan
   
    ErrorLog         logs/gerrit_error.log
    TransferLog     logs/gerrit_access.log
   
    AddDefaultCharset        On

    AllowEncodedSlashes NoDecode
    ProxyRequests Off
    ProxyVia Off
    ProxyPreserveHost On
    ProxyPass            /        http://lpsrvgit2:8080/ nocanon
    ProxyPassReverse     /        http://lpsrvgit2:8080/

</VirtualHost>
May 20, 2015
#12 sgunder...@bigfoot.com
Heh, seemingly the trick is the “nocanon” to ProxyPass. I wonder what that is about.
May 20, 2015
Project Member #13 edwin.ke...@gmail.com
At least it's documented that "nocanon" should be set for ProxyPass:
  https://gerrit-review.googlesource.com/Documentation/config-reverseproxy.html#_apache_2_configuration

Does it solve the problem for everyone? Can we close the ticket?
Status: AwaitingInformation
May 20, 2015
#14 zaharo...@gmail.com
Helped for us too! Many thanks!
May 20, 2015
#15 sgunder...@bigfoot.com
I haven't seen that documentation before; the only one I saw was Apache 2.4-specific, so we couldn't use it. (Not that I can find it again now...) But it also suggests AllowEncodedSlashes on, which broke viewing of commits last time I checked. Perhaps it's better when combined with nocanon?
May 20, 2015
#16 sgunder...@bigfoot.com
I tried; AllowEncodedSlashes on (even with nocanon) breaks side-by-side display of files with slashes in the filename.
Jun 19, 2015
#17 mikel...@aol.com
I've done more experimenting. If I have a file in the root of my repo (i.e. file.m) , I can diff side-by-side.
But if the file has even one level of depth to it (i.e. directory/file.m) then only unified diff works.

I will tell you what the problem was: in my config file, we were using mod_rewrite.
	# Access to anything under /gerrit/ is handled by tomcat which is running at 
	# localhost:8081.	Note that [P] implies [L], so stop here. 
--->	RewriteRule ^/gerrit/?(.*) http://localhost:8081/gerrit/$1 [P]
	
	# Outbound information from gerrit’s Jetty is rewritten to refer to /gerrit/ rather
	# than localhost:8081.
	ProxyPassReverse /gerrit/ http://localhost:8081/gerrit/


I changed the line above (marked with --->) to: ProxyPass /gerrit/ http://localhost:8081/gerrit/ nocanon

cleared my cache. 
Reloaded the site in a new browser window.
viola! I have side-by-side reviews working. 

So, what can we do if a site needs to use mod-rewrite rather than mod-proxy?

Only @sgunder can say if this can be closed (or not)

Thanks,
Mike




Sign in to add a comment

Powered by Google Project Hosting