Last Comment Bug 552239 - Camino doesn't build on 1.9.2
: Camino doesn't build on 1.9.2
Status: RESOLVED FIXED
[cm192test]
:
Product: Camino Graveyard
Classification: Graveyard
Component: General (show other bugs)
: 1.9.2 Branch
: All Mac OS X
: -- normal (vote)
: Camino2.1
Assigned To: Smokey Ardisson (offline for a while; not following bugs - do not email)
:
Mentors:
http://wiki.caminobrowser.org/User:Sa...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-03-13 19:13 PST by Smokey Ardisson (offline for a while; not following bugs - do not email)
Modified: 2010-04-08 11:36 PDT (History)
7 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Project changes, v1.0 (215.46 KB, patch)
2010-03-14 22:54 PDT, Smokey Ardisson (offline for a while; not following bugs - do not email)
stuart.morgan+bugzilla: superreview+
chris: feedback+
Details | Diff | Review
Add wallet .tbl files, v1.0 (59.65 KB, patch)
2010-03-15 19:37 PDT, Smokey Ardisson (offline for a while; not following bugs - do not email)
stuart.morgan+bugzilla: superreview+
Details | Diff | Review
Flashblock changes, v1.0 (3.05 KB, patch)
2010-03-15 19:44 PDT, Smokey Ardisson (offline for a while; not following bugs - do not email)
stuart.morgan+bugzilla: superreview+
Details | Diff | Review
{broken|loading}-image GIF -> PNG, v1.0 (1.77 KB, patch)
2010-03-15 20:23 PDT, Smokey Ardisson (offline for a while; not following bugs - do not email)
stuart.morgan+bugzilla: superreview+
Details | Diff | Review
pinstripe & embed-replacements changes, v1.0 (37.52 KB, patch)
2010-03-15 20:46 PDT, Smokey Ardisson (offline for a while; not following bugs - do not email)
stuart.morgan+bugzilla: superreview+
Details | Diff | Review
places chrome hackery removal, v1.0 (6.69 KB, patch)
2010-03-15 20:51 PDT, Smokey Ardisson (offline for a while; not following bugs - do not email)
stuart.morgan+bugzilla: superreview+
Details | Diff | Review
Assorted build files, v1.0 (13.35 KB, patch)
2010-03-15 21:47 PDT, Smokey Ardisson (offline for a while; not following bugs - do not email)
stuart.morgan+bugzilla: superreview+
Details | Diff | Review

Description Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-03-13 19:13:47 PST
The build changes are going to be fairly large and also fairly entangled; this bug is roughly going to cover changes to the mozconfig, confars.sh, Makefile, project, xcconfig, nsStaticComponents.cpp, and the trio of embed-replacements, pinstripe, and places (the camino/places buildhackery directory, not the history service itself).

I'll divide the changes into logical file-related patches where possible in order to make reviewing sane, but this bug will cover everything that's not a code-related change required to get a successful build.
Comment 1 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-03-13 19:27:18 PST
Note that there will also be additional fixes required to fix certain things (buildsymbols, cvs_tag, etc.) that will be regressions from 1.9.0 until follow-ups to this bug are filed and have fixes.
Comment 2 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-03-14 22:54:40 PDT
Created attachment 432497 [details] [diff] [review]
Project changes, v1.0

This is the project patch.  It won't build without other changes.  After a failed attempt at resolving merge conflicts from the places landing/xpautocomplete removal, I redid the changes.

Things seem OK on static; hendy, if you still have a tree that has all the other changes, can you swap this in for your project there and verify that debug builds, too.
Comment 3 Christopher Henderson 2010-03-15 13:34:00 PDT
Comment on attachment 432497 [details] [diff] [review]
Project changes, v1.0

Patching the project file in the test repo with this file results in a successful debug build.
Comment 4 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-03-15 19:37:20 PDT
Created attachment 432722 [details] [diff] [review]
Add wallet .tbl files, v1.0

This patch adds the wallet tables that we used to symlink from extensions/wallet/src/*.tbl into $objdir/wallet/tables to resources/application instead (since extensions/wallet did not come along for the mercurial ride).  They have a really fascinating cvs history, so we're not losing anything critical by just adding them.

I'm open to alternate locations if you feel like there's a better one (project patch will of course have to be updated), but all-camino.js and mimeTypes.rdf both live there, too, and all of these are siblings in Camino.app/Contents/MacOS/defaults/{type-of-default}/, so it seemed like the right place to me.
Comment 5 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-03-15 19:44:57 PDT
Created attachment 432723 [details] [diff] [review]
Flashblock changes, v1.0

These are the changes required to package and register Flashblock under the Toolkit chrome registry system.
Comment 6 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-03-15 20:23:04 PDT
Created attachment 432728 [details] [diff] [review]
{broken|loading}-image GIF -> PNG, v1.0

That most thrilling of patches, the git binary diff.  

hendy just converted our existing images from .gif to .png, since Core had changed their names.
Comment 7 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-03-15 20:46:46 PDT
Created attachment 432729 [details] [diff] [review]
pinstripe & embed-replacements changes, v1.0

These are the changes required to package our forked files.

Since embed-replacements no longer has the right folder structure to package things in the multi-jar toolkit idea of reality, I'm just extending our existing pinstripe jar packaging (remember, we hack-packaged toolkit theme files and modified license.html to not say MoCo on 1.9.0) to handle the relevant embed-replacements files.

It's a big patch, but the real meat of the change is the bottom of pinstripe/jar.mn.  intl.properties is now packaged for us by toolkit (as are the toolkit theme files, thus all the deletions in jar.mn), FAYT is gone, and the rest are old-chrome-registry contents.rdf files, also superfluous.
Comment 8 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-03-15 20:51:43 PDT
Created attachment 432730 [details] [diff] [review]
places chrome hackery removal, v1.0

This patch gets to remove the places/ chrome hackery that we just added to make Places work on 1.9.0; we get the required chrome registration and locale file packaging for free from toolkit.jar.
Comment 9 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-03-15 20:53:59 PDT
Comment on attachment 432729 [details] [diff] [review]
pinstripe & embed-replacements changes, v1.0

Er, I meant to add that we might want to clean up and merge the embed-replacements files with the pinstripe ones in the future, but fewer file moves are better during the bootstrapping stage.
Comment 10 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-03-15 21:15:42 PDT
Comment on attachment 432497 [details] [diff] [review]
Project changes, v1.0

Er, I meant to make this comment when posting the project patch:

libsydneyaudio links against CoreAudio, AudioToolbox, AudioUnit [9] but we only had to link against AudioUnit to link static; should we link the other two, also (and what about shared)?  What bad things might happen?

[9] http://mxr.mozilla.org/mozilla1.9.1/source/media/libsydneyaudio/src/Makefile.in#71
Comment 11 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-03-15 21:47:26 PDT
Created attachment 432733 [details] [diff] [review]
Assorted build files, v1.0

This is the last of the patches.

The Makefile.in changes:

1) places/ chrome-hack is gone (attachment 432730 [details] [diff] [review])

2) Fix the CFBundleVersion generation's pattern matching to account for minutes, seconds, and whatever else got added to the Build ID, which broke the original matching.

3) wallet tables live in resources/application now (attachment 432722 [details] [diff] [review])

4) embed-replacements files are packaged by pinstripe/jar.mn now (attachment 432729 [details] [diff] [review])

makefiles.sh: places/ chome-hack is gone

confvars.sh:

1) So far as I can tell, the build system no longer needs to pretend we're Mozilla.  mento was never clear as to what would break (embedding/config? xpfe?), but so far all seems well.

2) FAYT is gone

3) MOZ_XUL_APP=1 is implied by default now (but all the ifdefs are gone on 1.9.2)

4) We don't want the Mozilla Updater built.  This only suppresses building the stand-alone app; the chrome and in-app pieces are all built and packaged (as is xpinstall); I intend to file bugs on them.

build.mk: we're a real app now and don't need to build in embedding/config (it's gone on 1.9.3 and maybe 1.9.2, too)

app-config.mk: 

This is how we set defines that would ordinarily need their own configure logic (e.g., stuff that's not already defined in configure.in).  As you recall, all of our configure.in defines got ripped out in the great purge of whatever, so this is how we set MOZ_MACBROWSER (needed, for among other things, building osxspell).  It's the height of irony that the only examples of this file that exist were written by a certain someone.  

Note that only the SeaMonkey version includes the "rebuild-the-world" logic (see bug 381157 comment 26 for the rationale). I don't think we plan on adding more defines (but we could, for instance, build against 1.9.3 and 1.9.2 in one camino repo and with MOZILLA_1_9_2 ifdefs), but I'd rather have the logic in there from day 0 than ever need to add a define and waste a day trying to figure out why in the world it's not working, because I'll long have forgotten Callek's comment by then.

Camino.xcconfig: xmlextras died somewhere along the way.

nsStaticComponents.cpp:

1) XMLExtras died somewhere along the way.
2) FAYT is gone.

If I haven't forgotten anything, these (plus a couple of hendy's "fix-for-break-the-build" patches) will produce a complete working Camino build from the test repo.  I'll test that theory tomorrow.

Also, apologies for switching back and forth between cvs diffs and hg diffs against the test repo (wallet, broken/loading); hg was easier for the binary files, though for wallet I just forgot those files were already twiddled in cvs for the megapatch.
Comment 12 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-03-15 21:53:30 PDT
(In reply to comment #1)
> Note that there will also be additional fixes required to fix certain things
> (buildsymbols, cvs_tag, etc.) that will be regressions from 1.9.0 until
> follow-ups to this bug are filed and have fixes.

http://wiki.caminobrowser.org/User:Sardisson/Building_with_mozilla-central#To_do contains some of these; I'll try to push them into bugs at some point, but for now I'm catching my breath and focusing on shepherding the existing set of hendy's and my patches into the test-repo.
Comment 13 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-03-16 19:23:17 PDT
Comment on attachment 432497 [details] [diff] [review]
Project changes, v1.0

See also comment 10.
Comment 14 Stuart Morgan 2010-03-16 21:08:14 PDT
Comment on attachment 432722 [details] [diff] [review]
Add wallet .tbl files, v1.0

sr=smorgan
Comment 15 Stuart Morgan 2010-03-16 21:09:13 PDT
Comment on attachment 432728 [details] [diff] [review]
{broken|loading}-image GIF -> PNG, v1.0

sr=smorgan
Comment 16 Stuart Morgan 2010-03-16 21:11:23 PDT
Comment on attachment 432723 [details] [diff] [review]
Flashblock changes, v1.0

sr=smorgan
That certainly seems like a win for packaging specification :)
Comment 17 Stuart Morgan 2010-03-16 21:14:52 PDT
Comment on attachment 432729 [details] [diff] [review]
pinstripe & embed-replacements changes, v1.0

sr=smorgan
I won't pretend I understand jar packaging, but you haven't led us astray yet :)
Comment 18 Stuart Morgan 2010-03-16 21:15:39 PDT
Comment on attachment 432730 [details] [diff] [review]
places chrome hackery removal, v1.0

sr=smorgan. Best kind of patch!
Comment 19 Stuart Morgan 2010-03-16 21:25:25 PDT
(In reply to comment #10)
> libsydneyaudio links against CoreAudio, AudioToolbox, AudioUnit [9] but we only
> had to link against AudioUnit to link static; should we link the other two,
> also (and what about shared)?  What bad things might happen?

The source file they are compiling only includes the AudioUnit header, so it's unclear to me why they would link the other two. I would expect it would explode on link or launch if it needed the others, so I think we're fine with just AU until we have proof otherwise.

I'm not sure why we wouldn't need it for shared though; I would include it there as well.
Comment 20 Stuart Morgan 2010-03-16 21:36:33 PDT
Comment on attachment 432497 [details] [diff] [review]
Project changes, v1.0

sr=smorgan
Comment 21 Stuart Morgan 2010-03-16 21:43:38 PDT
Comment on attachment 432733 [details] [diff] [review]
Assorted build files, v1.0

sr=smorgan
Comment 22 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-03-16 21:47:03 PDT
(In reply to comment #19)
> I'm not sure why we wouldn't need it for shared though; I would include it
> there as well.

It occurred to me after making that comment that perhaps because we're not linking the library (.a) into Camino itself in the shared build (we're just juggling .dylibs around), we don't need to resolve the external symbols referenced by the library in shared; dyld and the dylib handle that for us.

Note that IOKit behaves the same way (it's only in static).
Here's where it entered Camino: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/camino/Chimera.pbproj/project.pbxproj&rev=&cvsroot=/cvsroot&mark=1355-1366#1355
Here's where it's used in Widget: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/cocoa/Makefile.in&rev=1.84&mark=180#180
They landed the same day.
Comment 23 Stuart Morgan 2010-03-16 22:17:30 PDT
Duh. Yes, the dylib will link it, and thus have the load commands, so we don't need to link it ourselves.

Why am I doing sr's again? ;)
Comment 24 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-03-16 22:55:11 PDT
(In reply to comment #23)
> Why am I doing sr's again? ;)

Because you actually knew the information required to identify the correct answer from the assorted answer-choices offered, whereas I was just guessing as to what was going on ;-)

Pushed all of these to the test repo: http://hg.mozilla.org/users/alqahira_ardisson.org/camino-1.9.2-test/rev/3d797c134820
Comment 25 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-04-08 11:36:31 PDT
http://hg.mozilla.org/camino/rev/1220357de22d

(Note, though, re: the summary of this bug, we don't build currently without bug 557992.)

Note You need to log in before you can comment on or make changes to this bug.