Page MenuHomePhabricator

Redirect git.wikimedia.org HEAD URLs to Diffusion
Closed, DeclinedPublic

Event Timeline

Nemo_bis created this task.Aug 3 2016, 7:34 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 3 2016, 7:34 AM
Paladox added a subscriber: Paladox.Aug 3 2016, 7:43 AM
Danny_B added a subscriber: Danny_B.Aug 3 2016, 8:39 AM

What is the desired target?

We should just remove HEAD from URL, ie strip it when it goes to the URL.

Change 302747 had a related patch set (by Paladox) published:
Strip out branch HEAD in git.wikimedia.org tree link

https://gerrit.wikimedia.org/r/302747

That's not the one I had in my mind. I was looking for the final redirect Phabricator URL (https://phabricator.wikimedia.org/r/p/<something>).

Is it presumable that HEAD should always link to master?

greg added a subscriber: greg.Aug 4 2016, 8:09 PM

Is it presumable that HEAD should always link to master?

The only odd one I know of is rOPUP (they use production as their master to be more clear)

Production is there default branch as master is in other repos, so it would work the same. I should have mention that I mean master or any default branch. (Background: Links without specified branches link to the default one, which is most typically master.)

So if the above applies, then it's trivial.

I can't submit a patch now, so just here (somebody please patch it on behalf of me, thanks):

https://phabricator.wikimedia.org/diffusion/OPUP/browse/production/modules/phabricator/templates/gitblit_vhost.conf.erb;5450db07e80c812d2cc9e0fef3a0f281c4698acc$43,44

Replace with:

# remove "HEAD/" and "refs%2Fheads%2F" and "refs%2Fremotes%2Forigin%2F"
RewriteRule ^(.*)(HEAD/|refs\%2[Ff](heads|remotes\%2[Ff]origin)\%2[Ff])(.*)$ $1$4
Danny_B added a subscriber: Dzahn.Aug 11 2016, 11:51 AM

@Dzahn (I guess you took care of the deployment of previous ruleset) - Can this be deployed or anything else is necessary? Thank you.

Dereckson updated the task description. (Show Details)Aug 13 2016, 7:53 PM
Nemo_bis updated the task description. (Show Details)Oct 7 2016, 1:19 PM
Dzahn added a comment.Oct 7 2016, 1:24 PM

Before it can be deployed it needs reviews on the Gerrit change from Phabricator/Gerrit maintainers.

Change 302747 merged by Dzahn:
phab/ex gitblit: Strip out branch HEAD in git.wikimedia.org tree link

https://gerrit.wikimedia.org/r/302747

Dzahn added a comment.Oct 25 2016, 2:02 AM

https://git.wikimedia.org/tree/translatewiki.git/HEAD/irc-relay currently redirects to >https://phabricator.wikimedia.org/diffusion/GTWN/browse/HEAD/irc-relay

Unhandled Exception ("DiffusionRefNotFoundException")
Ref "HEAD" does not exist in this repository.

This now changed to:

redirects to: https://phabricator.wikimedia.org/diffusion/GTWN/browse/irc-relay/

Unhandled Exception ("DiffusionRefNotFoundException")
Ref "irc-relay" does not exist in this repository.

This now changed to:
redirects to: https://phabricator.wikimedia.org/diffusion/GTWN/browse/irc-relay/
Unhandled Exception ("DiffusionRefNotFoundException")
Ref "irc-relay" does not exist in this repository.

Yay, close!
Removing the slash suffix (the last /) would fix this redirect example.

This seems like a bit of buggy url routing code in phabricator. The ending slash is allowed if you specify a branch name.

pathworks?
/browse/irc-relay
/browse/irc-relay/
/browse/master/irc-relay
/browse/master/irc-relay/

Offtopic here as this is only about HEAD:
Dealing with T137353 I've ran into numerous links not redirecting as expected. Want to allow providing feedback somewhere (?) whether worth considering too, or if they are just not common enough.

Thanks for the link updates and for this useful table.

It seems Diffusion breaks when trying to "Browse tags" on a repo that doesn't yet have tags.

For example, https://www.mediawiki.org/wiki/Extension:EventLogging#Download links to https://phabricator.wikimedia.org/r/p/mediawiki/extensions/EventLogging;tags/master/ which we redirect to https://phabricator.wikimedia.org/diffusion/EEVL/tags/master/. Then then fails with:

Unhandled Exception ("RuntimeException")	
Undefined variable: view

By comparison https://phabricator.wikimedia.org/r/p/oojs/ui;tags/master/ redirects to https://phabricator.wikimedia.org/diffusion/GOJU/tags/master/ and works fine.

greg added a comment.Sep 26 2017, 4:25 PM

It seems Diffusion breaks when trying to "Browse tags" on a repo that doesn't yet have tags.
For example, https://www.mediawiki.org/wiki/Extension:EventLogging#Download links to https://phabricator.wikimedia.org/r/p/mediawiki/extensions/EventLogging;tags/master/ which we redirect to https://phabricator.wikimedia.org/diffusion/EEVL/tags/master/. Then then fails with:

Unhandled Exception ("RuntimeException")	
Undefined variable: view

This now works and you are shown a "This repository has no tags." message in the normal UI (ie, not a weird exception).

Aklapper lowered the priority of this task from Normal to Low.Sep 27 2017, 12:22 PM
Krinkle removed a subscriber: Krinkle.
mmodell closed this task as Declined.Oct 3 2017, 11:20 PM

I don't actually see tags/master/ anywhere on the page https://www.mediawiki.org/wiki/Extension:EventLogging (I opened view source and did a text search, it's nowhere to be found)

At this point, gitblit on git.wikimedia.org is ancient history. There shouldn't be many links remaining which point to the old URLs and even if a few remain then I don't think it's a huge injustice to leave them broken.