471318 - Many revision urls on waterfall are "wrong"

Status: VERIFIED FIXED

(Release Engineering :: General, defect, P3)

Description

[Serge Gautherie (:sgautherie)]

1) (talos boxes seem fine.)

2) new 1.9.1 unit test column misses the "releases/" part.

(mozilla-central unit test are fine.)

Example:
{
TinderboxPrint: "<a href=http://hg.mozilla.org/mozilla-1.9.1/index.cgi/rev/4594d93b4d54 title=\"Built from revision 4594d93b4d54\">rev:4594d93b4d54</a>"
}

3) '/' is doubled after the hostname.

This affects "all" the other boxes, on FF/SM/TB, for both mozilla* and comm-central repos...

Example
{
TinderboxPrint: <a href=http://hg.mozilla.org//mozilla-central/index.cgi/rev/bc174f0cc10d title="Built from revision bc174f0cc10d">rev:bc174f0cc10d</a>
}

Comment 1

[Chris Cooper [:coop] (he/him)]

Lukas: did this get munged when we merged unittest with the regular builders? Should be as simple as fixing some paths in the configs, I imagine.

Comment 2

[Serge Gautherie (:sgautherie)]

4) extra 'index.cgi/'

This part is (no longer ?) wanted.

Comment 3

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: use self.repoPath instead of self.branch for Tinderbox revision URL

Comment 4

[Serge Gautherie (:sgautherie)]

Comment on attachment 355618 [details] [diff] [review]
use self.repoPath instead of self.branch for Tinderbox revision URL

>-            self.branch,
>+            self.repoPath,
>             '/index.cgi/rev/%(got_revision)s title="Built from revision %(got_revision)s">rev:%(got_revision)s</a>'])

Why not remove '/index.cgi', here and at another place in this file ?
(per comment 2)

Comment 5

[[e-mail] (:bhearsum)]

Comment on attachment 355618 [details] [diff] [review]
use self.repoPath instead of self.branch for Tinderbox revision URL

Checking in process/factory.py;
/cvsroot/mozilla/tools/buildbotcustom/process/factory.py,v  <--  factory.py
new revision: 1.73; previous revision: 1.72
done

Comment 6

[[e-mail] (:bhearsum)]

Alright, the master has been updated. New builds should be OK.

Comment 7

[Serge Gautherie (:sgautherie)]

Afaict, this solved point 2 :-)
but not 3 and 4...

Comment 8

[[e-mail] (:bhearsum)]

It works now. Patches accepted for the rest.

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Addressing points 3 & 4 for tinderbox urls

I've removed the index.cgi from both unittest and mercurial revision URLs, and also taken away the additional '/'

Comment 10

[Chris Cooper [:coop] (he/him)]

reassigning back to lukas, since there's a patch pending.

Comment 11

[[e-mail] (:bhearsum)]

Comment on attachment 355795 [details] [diff] [review]
Addressing points 3 & 4 for tinderbox urls

I don't see an extra slash in the unittest links. I think this patch will break them. Have you tested it?

Comment 12

[Serge Gautherie (:sgautherie)]

(In reply to comment #0)
> 3) '/' is doubled after the hostname.
> 
> This affects "all" the other boxes, on FF/SM/TB, for both mozilla* and
> comm-central repos...

Point 3 and 4 were fixed for SM by bug 472289 :-)

Comment 14

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Removes index.cgi from the URL

This patch only removes index.cgi from the url since it's unnecessary.  I can't find links in Tinderbox that have the double slash. If that's still an issue, please give an example.

Comment 15

[Chris Cooper [:coop] (he/him)]

Comment on attachment 379586 [details] [diff] [review]
Removes index.cgi from the URL

(In reply to comment #14) 
> This patch only removes index.cgi from the url since it's unnecessary.  I can't
> find links in Tinderbox that have the double slash. If that's still an issue,
> please give an example.

I *think* MozillaBuildFactory.getRepository() handles stripping the extra '/' from the host/repo now, provided we actually use that function everywhere.

Style nit: can you line up the args again to changesetLink in MercurialBuildFactory now that you've shortened the line?

Comment 16

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Removes index.cgi from the URL

Comment 17

[Chris Cooper [:coop] (he/him)]

Comment on attachment 379600 [details] [diff] [review]
Removes index.cgi from the URL

changeset:   314:c030d9bc0b92

Comment 18

[Lukas Blakk [:lsblakk] use ?needinfo]

Production master was reconfig'd so this should clear up in the next round of builds.

Comment 20

[Serge Gautherie (:sgautherie)]

(In reply to comment #19)

This bug is V.Fixed as it was initially reported.

> *** Bug 478127 has been marked as a duplicate of this bug. ***

Yet, the occurrences from that other bug remain to be fixed.

Comment 21

[Lukas Blakk [:lsblakk] use ?needinfo]

I've looked at Tinderbox and can't find any occurrences that need fixing, please specify what issues remain before this can be closed.

Comment 22

[Serge Gautherie (:sgautherie)]

(In reply to comment #21)
> I've looked at Tinderbox and can't find any occurrences that need fixing,

Bug 478127 was not about Tinderbox...

> please specify what issues remain before this can be closed.

Just follow the url!
http://mxr.mozilla.org/build/search?string=index%5C.cgi&regexp=on&case=on

Comment 23

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Removes index.cgi from buildbot-configs - various config files

Comment 24

[Aki Sasaki (not active)]

Comment on attachment 382129 [details] [diff] [review]
Removes index.cgi from buildbot-configs - various config files

revision 1200:28ecae30d384

Comment 25

[Lukas Blakk [:lsblakk] use ?needinfo]

Checked in and the Tinderbox URLs look good.

Comment 26

[Serge Gautherie (:sgautherie)]

V.Fixed, at last.