Bug 538890 broke about:buildconfig unification

RESOLVED FIXED in mozilla2.0b7

Status

()

Toolkit
Build Config
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: Smokey Ardisson (offline for a while; not following bugs - do not email), Assigned: Smokey Ardisson (offline for a while; not following bugs - do not email))

Tracking

(Blocks: 1 bug, {regression})

Trunk
mozilla2.0b7
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

Apparently no-one ever tested the patch for bug 538890; it breaks buildconfig unification, and as a result, an <hr> and ugly x86_64 buildconfig info is all that's shown.

I think I have a fix, but since you have to be on 10.6 to build a Universal Binary any more, I've pushed my patch to try.

Comment 2

8 years ago
These empty <p>s were present in the original file introduced in 2003:
http://mxr.mozilla.org/mozilla/source/xpfe/global/buildconfig.html.in
They used to add some margin before the <h2> elements.

There's no script in this file, nor any hint in the cvs/hg history that there's a "buildconfig unification", let alone where that happens.

So, this happens here:
> Copy the second file's content beginning after its leading <h1> and <p>.
http://mxr.mozilla.org/mozilla-central/source/build/macosx/universal/fix-buildconfig#129
The empty <p>s are not even needed by fix-buildconfig.

Please just fix that to not look for the first <p>, but the first </h1> instead, i.e. the <h1>about:buildconfig</h1> line, and don't add the hacky and unneeded empty <p>s back.
Thanks for filing this, and figuring out the regression. I noticed this recently but didn't have time to figure out what regressed it.
Created attachment 479131 [details] [diff] [review]
Actual fix

I figured out a way to hack this up locally to test, so this should be the complete fix.  Patch is currently going to try for sanity-check, since "fix based on local hack" doesn't inspire as much confidence as I'd like, and since try has caught other gotchas for me in this now-two-bug process. ;)

This patch does four things:

1) Updates fix-buildconfig to not depend on the <p> </p> lines.
2) Since I'm cleaning up after bug 538890 here already (and also had these changes in my patch for bug 600133), this fixes cls and biesi to match every single other instance in the tree of them as contributors.
3) Removes the blank line that also breaks fix-buildconfig
4) Adds a warning to the file so that future modifiers will remember to test changes in a Universal binary, so that hopefully we'll never find ourselves in this state ever again ;)

I'll request review once there are working bits hot off try.
Attachment #478980 - Attachment is obsolete: true
(In reply to comment #4)
> changes in my patch for bug 600133), this fixes cls and biesi to match every
Make that bug 456360.
Comment on attachment 479131 [details] [diff] [review]
Actual fix

http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/alqahira@ardisson.org-8f549fc87341/tryserver-macosx64/firefox-4.0b7pre.en-US.mac64.dmg displays the correct about:buildconfig again :)

>    # Copy the second file's content beginning after its leading <h1> and <p>.
Already fixed that comment locally to remove the ' and <p>' part that no longer is correct post-bug 538890 having removed the empty <p>s.

Gavin promised me last night he'd take duties on this if I fixed it.
Attachment #479131 - Flags: review?(gavin.sharp)
Attachment #479131 - Flags: approval2.0?
Comment on attachment 479131 [details] [diff] [review]
Actual fix

I'd mention fix-buildconfig specifically in the comment, r=me with that.
Attachment #479131 - Flags: review?(gavin.sharp)
Attachment #479131 - Flags: review+
Attachment #479131 - Flags: approval2.0?
Attachment #479131 - Flags: approval2.0+
http://hg.mozilla.org/mozilla-central/rev/7e7c7a311e0d (with an improved comment mentioning fix-buildconfig explicitly)
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
Target Milestone: --- → mozilla2.0b8

Updated

7 years ago
Target Milestone: mozilla2.0b8 → mozilla2.0b7
You need to log in before you can comment on or make changes to this bug.