New issue
Advanced search Search tips
Starred by 15 users

Issue metadata

Status: Released
Owner:
Closed: Nov 14
Cc:
Components:
ReleasedIn: 522.0



Sign in to add a comment
link

Issue 5316: Gitiles links do not work because they use wrong relative paths

Reported by jkt@flaska.net, Jan 22 2017

Issue description

Affected Version:

- Gerrit 640381f50ddfe5c2c73e36d4fafae731678f0202 with Ic2df0f270c178e7a93dde5d0460dd24f10edf321 and I8b674550e34b0c6fed4cc0af2f069aaeadbae6cc applied

- Gitiles 7239ac50bfe6ceda5fa82a37b57ed9b7a6aa7139

When I'm on https://gerrit.cesnet.cz/c/5/ , there's a "Links" field with a "browse" text which points to gitiles, but the produced URL is https://gerrit.cesnet.cz/c/5/plugins/gitiles/dummy/+/c51c13c3a2625349018521f051e652ef04d7eac0 . That doesn't work, the "/c/5" should not be there.

In addition, the patchset view shows a short hash of the commit which was merged, but it is not clickable. That thing is clickable on gerrit-review.googlesource.com.
 

Comment 1 by david.pu...@gmail.com, Jan 23 2017

Status: AwaitingInformation (was: New)
When I go to https://gerrit.cesnet.cz/c/5/ the "browse" link is:

   https://gerrit.cesnet.cz/plugins/gitiles/dummy/+/c51c13c3a2625349018521f051e652ef04d7eac0

> the patchset view shows a short hash of the commit which was merged, but it is not clickable

Can you elaborate where this is?  A screenshot would help.

Comment 2 by david.pu...@gmail.com, Jan 23 2017

Status: New (was: AwaitingInformation)
Sorry, disregard my previous comments.  I missed that this was for Polygerrit, and I was testing the GWT version...

Comment 3 by jkt@flaska.net, Jan 23 2017

The attached picture shows what gets turned into a link on googlesource, but not on my instance of Polygerrit.
gerrit-5316-sshot.png
48.8 KB View Download

Comment 4 by andyb...@chromium.org, Jan 23 2017

Project Member
Status: Accepted (was: New)

Comment 5 by fastest...@gmail.com, Apr 24 2017

I'm running into this with 2.14-rc0. The "browse" anchor's source is "plugins/gitiles/ansible-repo/+/27e78b687c8dc17e01fe35c4cdd1e153ea942b23" which, when clicked, opens "/c/5/plugins/gitiles/ansible-repo/+/27e78b687c8dc17e01fe35c4cdd1e153ea942b23" which is not correct.

Comment 6 by fastest...@gmail.com, Jun 21 2017

Any update on this? I would assume it's a small change but is extremely frustrating to not have the browse link work.

Comment 7 by wyatta@google.com, Jun 21 2017

Project Member
Status: AwaitingInformation (was: Accepted)
Is the browse link setup using a weblinks section of that server's Gerrit config file? If so, is it possible that the URL pattern in the "browse" link config needs a slash prepended to it?

Comment 8 by drifk...@gmail.com, Aug 1 2017

I believe there is a fix for this issue that needs review at https://gerrit-review.googlesource.com/#/c/plugins/gitiles/+/104783/

Comment 9 by jkt@flaska.net, Sep 23 2017

It seems that there are two issues that are getting mixed in this report. The first one of them is that the Gitiles plugin currently only produces relative URLs which do not work. That's being addressed by https://gerrit-review.googlesource.com/c/plugins/gitiles/+/104783 .

There is also other problem which is illustrated by Comment 3. On a Polygerrit change screen, the commit SHA in between the patchset selection and the Download link is not clickable. Given that it *is* clickable on gerrit-review.googlesource.com, it is probably due to some misconfiguration on my side. I'm running with no config for the gitiles plugin, and my server's error log says that the plugin is using its defaults. The documentation says that noWebLinks defaults to false.

> Is the browse link setup using a weblinks section of that server's Gerrit config file? If so, is it possible that the URL pattern in the "browse" link config needs a slash prepended to it?

I am a bit lost here -- the documentation does not contain anything about "weblinks" as far as I can tell. Do I have to enable something to get a clickable link here? This is not about a "commentlink", is it? I also do not have any configuration related to "gitweb" in case that might matter.

Comment 10 by kaspern@google.com, Apr 20 2018

Project Member
Is this still not working?

Comment 11 by gertvdijk@gmail.com, Jun 4 2018

Project Member
Yes this is still an issue, also on master.

I wanted to point out that the 'browse' link on the file pages are OK, but just wrong on the change page.

So, with Gerrit running under the webroot /, the link generated on a change page for project 'mycategory/myrepo' (e.g. /c/mycategory/myrepo/+/1234) is (wrongfully):

hxxps://gerrit.mydomain.tld/c/mycategory//plugins/gitiles/mycategory/myrepo/+/<hash>

That makes 'c/mycategory//' superfluous in this URL (and yes this is somehow 'missing' the 'myrepo' part).

When viewing the file diff on a page, the link to the file in Gitiles is generated OK:

hxxps://gerrit.mydomain.tld/plugins/gitiles/mycategory/myrepo/+/refs/changes/34/1234/1/path/to/file

Versions used:
gerrit 2.15.2-3879-g436b57ea85 from gerrit-ci
gitiles fd51b3f2b9 from gerrit-ci as well

Comment 12 by logan@google.com, Jun 6 2018

Project Member
If you configure an absolute URL for gitweb.url (see https://gerrit-review.googlesource.com/Documentation/config-gerrit.html#gitweb.url), does that resolve this problem?

Comment 13 by logan@google.com, Jun 6 2018

Project Member
 Issue 8656  has been merged into this issue.

Comment 14 by gertvdijk@gmail.com, Jun 6 2018

Project Member
@logan:
I should note that I haven't installed gitweb in my installation. Are you intentionally pointing to a *gitweb* and not *gitiles* setting - and don't you mean the gitiles.config gerrit.baseUrl setting [1] instead?

With gerrit.config gitweb.url set, nothing changes for Gitiles in my installation. (Same versions used as previous comment.)

I've tried adjusting the gitiles.config gerrit.baseUrl setting and restarted Gerrit, but this only has effect on the change page 'browse' URL generated, *not* on those when viewing a file. Sounds like a(n) (un)related bug?

However, I tried to outsmart the current behaviour here to make it working for my situation by setting the base path to the default but absolute /plugins/gitiles path:

gitiles.config:
[gerrit]
    baseUrl = https://gerrit.mydomain.tld/plugins/gitiles

And now my browse links work everywhere, but this is clearly not how it should be and I still consider this a bug.

Hope this helps.

[1]: https://gerrit.googlesource.com/plugins/gitiles/+/HEAD/src/main/resources/+Documentation/config.md#file

Comment 15 by logan@google.com, Jun 6 2018

Project Member
Labels: Triaged-Yes Hotlist-Linkification Hotlist-GWT Priority-2
Owner: wyatta@google.com
Status: Accepted (was: AwaitingInformation)
My thoughts went to gitweb config because some source browsing plugins use it, but I see now that gitiles has its own config. In any case, I'm glad you found/confirmed a workaround.

What we could do to fix this on PolyGerrit's end is detect when we're given a relative URL for any gitweb or gitiles link, and do the right thing with it to produce an absolute URL. Unfortunately I think this logic is scattered across a number of different components, dealing with a number of different kinds of links.

Comment 16 by marceloa...@gmail.com, Jun 6 2018

I have the same issue in Gerrit 2.15.2 but using GitBlit instead of Gitiles. The Links field in the change points to:

https://SERVER/c/REPO/+/plugins/gitblit/commit/?r=REPOd&h=COMMIT

Instead of:

https://SERVER/plugins/gitblit/commit/?r=REPOd&h=COMMIT

I have gitweb.url = https://SERVER/plugins/gitblit/ set in the GERRIT_SITE/etc/gerrit.config file.

Comment 17 by john.vik...@effnet.com, Jun 7 2018

The workaround to explicitly set the url worked for me.
I also had to flush the cahces. Ie: ssh -p <port> <host> gerrit flush-caches --all

Comment 18 by marceloa...@gmail.com, Jun 7 2018

@john.vik...@effnet.com, Are you saying that you set the absolute url, flush the caches and the links field in the change, using PolyGerrit, doesn't add "c/REPO/+" in the link anymore? Is it GitBlit or Gitiles? Which is the Gerrit version?

Comment 19 by john.vik...@effnet.com, Jun 7 2018

@marceloa...@gmail.com yes that is correct, I added url = http://xyz/gitweb to the [gitweb] section in the configuration and flushed the cache. The flush was necessary since the old page with the incorrect /c/ url was cached after I changed the configuration.
I'm using gitweb on gerrit 2.15.2.

Comment 20 by marceloa...@gmail.com, Jun 11 2018

Sorry everyone, I think it was my browser cache :-( The workaround (set the absolute URL using gitweb.url = https://SERVER/plugins/gitblit/ at GERRIT_SITE/etc/gerrit.config file worked pretty well in every "GitBlit" links.

Comment 21 by sven.sel...@axis.com, Jun 28 2018

Project Member
One issue is that PolyGerrit does not trust the WebLink provider to provide a correct relative URL and tries to second-guess how the WebLink url should be presented.

https://gerrit-review.googlesource.com/c/gerrit/+/186875

Comment 22 by wyatta@google.com, Jul 3 2018

Project Member
ReleasedIn: 522.0

Comment 23 by wyatta@google.com, Jul 9 2018

Project Member
Status: Released (was: Accepted)

Comment 24 by david.pu...@gmail.com, Jul 18 2018

Labels: FixedIn-2.15.3

Comment 25 by gertvdijk@gmail.com, Jul 19 2018

Project Member
Is this really fixed? It's appears not for me at least. I just tested it on master and the behavior is the same as before.

* Verified that https://gerrit-review.googlesource.com/c/gerrit/+/186875 is included in master now.
* Upgraded to master/2.15.3-4415-ga24fb63 from gerrit-ci.
* Using gitiles plugin version be8c047 from gerrit-ci.
* Undone my workaround in gitiles.config / gerrit.baseUrl setting.

Generates wrong URLs on 'browse' links, exactly the same as I mentioned in comment 11.

Restoring the workaround in gitiles.config with the new versions generates the right URLs again.

Comment 26 by kaspern@google.com, Jul 20 2018

Project Member
Labels: -Triaged-Yes Triaged-No
Status: New (was: Released)
Repoening as per report.

Comment 27 by wyatta@google.com, Aug 3

Project Member
Labels: -Triaged-No Triaged-Undecided

Comment 28 by gertvdijk@gmail.com, Aug 19

Project Member
Components: plugins>gitiles

Comment 29 by david.pu...@gmail.com, Sep 7

Labels: FixedIn-2.14.12

Comment 30 by david.pu...@gmail.com, Sep 7

Labels: -Triaged-Undecided Triaged-Yes

Comment 31 by david.pu...@gmail.com, Sep 7

Labels: Type-Bug
Owner: ----
Status: Submitted (was: New)

Comment 32 by david.pu...@gmail.com, Sep 10

Status: Released (was: Submitted)

Comment 33 by david.pu...@gmail.com, Nov 8

Status: New (was: Released)

Comment 34 by duft.mar...@gmail.com, Nov 8

I'm slightly confused by the labels and state changes of the issue :) Should this work on current master? As reported on the mailing list:

We're using the gitiles plugin. Clicking (browse) in the repository list view (or on a specific branch in the branch list) brings me to the correct gitiles page. However when using (browse) on any change/commit directs to a wrong URL

e.g. it generates

 hxxps://git.ssi-schaefer.com/c/products//plugins/gitiles/products/wamas/+/a37b3316fc67f50d2e2e3dd32ffc026692f2a54e

where it should have been

 hxxps://git.ssi-schaefer.com/plugins/gitiles/products/wamas/+/a37b3316fc67f50d2e2e3dd32ffc026692f2a54e‚Äč

I am on current master (from today morning) with current gitiles plugin (not much happening there anyway :)). I did NOT set any URL configs, etc. up until now.

Comment 35 by david.pu...@gmail.com, Nov 13

Status: Accepted (was: New)
I've just tested it on the latest master branch of the plugin against gerrit stable-2.16, and it doesn't work.

Comment 36 by david.pu...@gmail.com, Nov 13

As far as I can see, the plugin is generating the correct URL, i.e. "/plugins/gitiles/project/+/sha1" but in the PG UI it ends up with "/c/".

Comment 37 by david.pu...@gmail.com, Nov 13

In other words, this will affect all plugins that implement the weblinks interface, not only gitiles.

Comment 38 by david.pu...@gmail.com, Nov 13

Labels: Blocking-2.16

Comment 39 by david.pu...@gmail.com, Nov 13

Cc: kaspern@google.com

Comment 40 by kaspern@google.com, Nov 13

Project Member
Cc: -kaspern@google.com
Owner: kaspern@google.com

Comment 41 by kaspern@google.com, Nov 13

Project Member
Status: ChangeUnderReview (was: Accepted)
https://gerrit-review.googlesource.com/c/gerrit/+/204275

Comment 42 by kaspern@google.com, Nov 14

Project Member
Status: Submitted (was: ChangeUnderReview)

Comment 43 by luca.mil...@gmail.com, Nov 20

Project Member
Labels: FixedIn-2.16
Status: Released (was: Submitted)

Comment 44 by sven.sel...@axis.com, Jan 21

Project Member
 Issue 10344  has been merged into this issue.

Comment 45 by alon.bar...@gmail.com, Jan 21

Hi,
I am unsure how  Issue 10344  is duplicate of this issue while it happens in 2.16.2 and this bug is reported to be fixed in 2.16.
Thanks!

Comment 46 by david.pu...@gmail.com, Jan 21

Either the bug was not fully fixed (i.e.  issue 10344  is another case that was missed), or there is a regression and it's been broken again since 2.16.

Comment 47 by gertvdijk@gmail.com, Jan 22

Project Member
I've just tested this on a clean site with Gerrit 2.16.3 and Gitiles, both from Gerrit-CI [1], [2], no gitiles config, all default. Result: "Works for me."

So, I'm not sure what's specific to your site, Alon. Or are you looking at a stale browser cache perhaps? (That has bitten me very often; Ctrl+F5 is not sufficient.)

[1]: https://gerrit-releases.storage.googleapis.com/gerrit-2.16.3.war
[2]: https://gerrit-ci.gerritforge.com/view/Plugins-stable-2.16/job/plugin-gitiles-bazel-master-stable-2.16/13/artifact/bazel-genfiles/plugins/gitiles/gitiles.jar (reported version e93fe97681)

Comment 48 by gertvdijk@gmail.com, Jan 22

Project Member
Ah, looking at I7d3663c8752442887ad6b95b49d515098ea21d76 for  Issue 10344 , it seems that this is only triggered if you're not running Gerrit on '/' on your domain, but on a base path. I missed that setting a canonicalWebUrl to something that includes a base path is crucial for  Issue 10344  (I thought it was just an example). FWIW, IMHO, it's okay to keep the two issues separate; we had apparently two related, but different issues with plugin URLs generated.

Comment 49 by sven.sel...@axis.com, Jan 22

Project Member
I regarded it as a duplicate since the issue was fixed in the context of this issue
I have done the same tests with a canonicalWebUrl and a context path and it works for me. Gerrit: v2.16.3 gitiles: e93fe97

The statement:
"Please notice that if the url starts with '/' it should also trim the path component from this.getBaseUrl() to reach something that makes sense."
In the initial issue makes me believe that you've added the context path of the canonicalWebUrl to the gitiles link.

PolyGerrit assumes that:
* the weblink provides it's own absolute path
or
* the weblink provides a path relative to canonicalWebUrl

What does your gitiles.config look like?
Would I be right in guessing that it is something in the lines of:

[gerrit]
   baseUrl = /gerrit/plugins/gitiles

Comment 50 by alon.bar...@gmail.com, Jan 22

The gitiles.config should be empty, the gitiles plugin has all the required information to construct the URL.

Comment 51 by sven.sel...@axis.com, Jan 22

Project Member
My point exactly.

Comment 52 by sven.sel...@axis.com, Jan 31

Project Member
Can you confirm whether the issue was due to faulty configuration in gitiles.config?

Comment 53 by alon.bar...@gmail.com, Jan 31

Hi,

Which issue? This or  Issue 10344 ?

I can confirm that my etc/gitiles.config contains 0 bytes.

I can also confirm that the code in master[1] does not make sense even when reading it. The link gitiles returns is full link /prefix/gitiles while the code will prefix again.

Please note that all gitiles links are working correctly but change link.

Thanks!

[1] https://gerrit.googlesource.com/gerrit/+/master/polygerrit-ui/app/elements/core/gr-router/gr-router.js#307

Sign in to add a comment