Closed Bug 471318 Opened 13 years ago Closed 13 years ago

Many revision urls on waterfall are "wrong"

Categories

(Release Engineering :: General, defect, P3)

defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: sgautherie, Assigned: lsblakk)

References

()

Details

Attachments

(3 files, 2 obsolete files)

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>
}
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.
Priority: -- → P3
4) extra 'index.cgi/'

This part is (no longer ?) wanted.
Assignee: nobody → lukasblakk
Status: NEW → ASSIGNED
Attachment #355618 - Flags: review?(bhearsum)
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 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
Attachment #355618 - Flags: review?(bhearsum) → review+
Attachment #355618 - Flags: checked‑in+ checked‑in+
Alright, the master has been updated. New builds should be OK.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Afaict, this solved point 2 :-)
but not 3 and 4...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
It works now. Patches accepted for the rest.
Assignee: lukasblakk → nobody
I've removed the index.cgi from both unittest and mercurial revision URLs, and also taken away the additional '/'
Attachment #355795 - Flags: review?(bhearsum)
reassigning back to lukas, since there's a patch pending.
Assignee: nobody → lukasblakk
Status: REOPENED → ASSIGNED
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?
Attachment #355795 - Flags: review?(bhearsum) → review-
(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 :-)
Depends on: 472289
Depends on: 471317
Blocks: 478127
Duplicate of this bug: 478127
No longer blocks: 478127
Blocks: 478127
Attached patch Removes index.cgi from the URL (obsolete) — Splinter Review
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.
Attachment #355795 - Attachment is obsolete: true
Attachment #379586 - Flags: review?(ccooper)
Attachment #379586 - Flags: review?(ccooper) → review+
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?
Attachment #379586 - Attachment is obsolete: true
Attachment #379600 - Flags: checked‑in? checked‑in?
Comment on attachment 379600 [details] [diff] [review]
Removes index.cgi from the URL

changeset:   314:c030d9bc0b92
Attachment #379600 - Flags: checked‑in? checked‑in? → checked‑in+ checked‑in+
Production master was reconfig'd so this should clear up in the next round of builds.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
No longer blocks: 478127
Duplicate of this bug: 478127
(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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I've looked at Tinderbox and can't find any occurrences that need fixing, please specify what issues remain before this can be closed.
(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
Status: REOPENED → ASSIGNED
Attachment #382129 - Flags: review?(ccooper) → review+
Attachment #382129 - Flags: checked‑in? checked‑in?
Attachment #382129 - Flags: checked‑in? checked‑in? → checked‑in+ checked‑in+
Comment on attachment 382129 [details] [diff] [review]
Removes index.cgi from buildbot-configs - various config files

revision 1200:28ecae30d384
Checked in and the Tinderbox URLs look good.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
V.Fixed, at last.
Status: RESOLVED → VERIFIED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.