383934 - Re-organize targets to allow preference panes to use -bundle_loader

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

[Stuart Morgan]

Right now we have several places where we are either compiling duplicate code into preferences panes, or we are doing ugly workarounds (including copying and pasting methods into the pref pane class) to give the pref panes access to code that's compiled into Camino anyway. I'm running into that pretty hard trying to switch the preferences to the new Cocoa nsIPermissionManager wrapper.

I think we should re-organize the targets so that we have one that builds just the Camino executable, so that we can reference it with -bundle_loader in the preference panes, then another that bundles all the pref panes into a full Camino.app. It should make pref pane development easier and potentially make them a bit smaller.

Thoughts?

Comment 1

[Mark Mentovai]

Yes, this should keep things cleaner.  I hope that this won't be complicated by having both Camino and CaminoStatic targets, though.

Comment 2

[Mark Mentovai]

I need this for bug 354975, so I'm taking it and doing it.  Stuart assures me he doesn't mind.

Comment 3

[Stuart Morgan]

Sweet, then I don't have to hack my pref changes after all.

Comment 4

[Mark Mentovai]

Attachment: Use aggregate targets to build the application; set bundle loader for preference panes

Use aggregate targets to build the application; set bundle loader for preference panes

The high-level overview here: I introduce new aggregate targets (ordered lists of other targets) to ensure that we can build the application and the preference panes without either having a direct dependency on the other as far as Xcode is concerned.  The application's target no longer has a CopyFiles phase to copy preference panes into place.  Instead, preference pane bundles are built directly in the right spot in the application (Camino.app/Contents/PreferencePanes).  Preference panes specify that Camino.app/Contents/MacOS/Camino is their bundle loader, and therefore are now able to link against anything in the main application bundle.  The aggregate targets build the preference panes after the main application in order for the application to be available for ld's use as a -bundle_laoder when the panes are linked.

More details: the existing application targets Camino and CaminoStatic have been renamed to CaminoApp and CaminoStaticApp.  The new aggregate targets are named Camino and CaminoStatic.  There's no change to the makefiles, because the aggregate targets (going by the names that the application targets used to have) are what we want to build: they'll build the world and produce properly-functioning complete Camino.apps in the same spot as always in the build directory.

An added benefit of this reorganization is that I was able to move PreferencePaneBase into the application, rather than having each preference pane compile its own copy.  Incidentally, that fixes bug 185954.  I wanted to rename PreferencePaneBase to CmPreferencePane, but I was already doing lots of other stuff that resulted in a huge patch, so I decided against it for now.

Here are the notes on the project file changes.  This'll help us come up with a branch version of the patch:

 - delete prefpane dependencies from Camino, CaminoStatic
 - delete Copy Preference Pane phases from Camino, CaminoStatic
 - rename Camino to CaminoApp, CaminoStatic to CaminoStaticApp
 - sort prefpanes in target list as:
   Navigation Appearance Privacy Security History Tabs Downloads WebFeatures
 - add Camino target as Aggregate, include CaminoApp and prefpanes
 - add CaminoStatic target as Aggregate, include CaminoStaticApp and prefpanes
 - set default target to Camino
 - fix PreferencePaneBase.h,mm to point to files in new location
   (src/preferences)
 - add PreferencePaneBase.h to CaminoApp,CaminoStaticApp's Copy Headers
 - add PreferencePaneBase.mm to CaminoApp,CaminoStaticApp's Compile Sources
 - remove PreferencePaneBase.mm from each preference pane
 - ensure PreferencePaneBase.h is in each preference pane's Copy Headers
 - remove PreferencePanes.framework from each preference pane
 - remove all Gecko libs from each preference pane, except:
    + leave libxpcom_core.dylib in Privacy, History, Downloads, Web Features
 - remove Carbon.framework from Appearance, Navigation, History, Tabs
 - clear framework, header, and library search paths from all build
   configurations for all preference panes, except:
    + framework search path
       * Navigation: sparkle/build/releaswe
    + header search path
       * Privacy: ../dist/include ../dist/include/cookie
                  ../dist/include/necko ../dist/include/nspr
                  ../dist/include/pref ../dist/include/string
                  ../dist/include/xpcom
       * Security: ../dist/include ../dist/include/nspr ../dist/include/pref
                   ../dist/include/xpcom
       * History: ../dist/include ../dist/include/docshell
                  ../dist/include/history ../dist/include/necko
                  ../dist/include/nkcache ../dist/include/nspr
                  ../dist/include/xpcom
       * Tabs: ../dist/include ../dist/include/dom ../dist/include/nspr
               ../dist/include/xpcom
       * Downloads: ../dist/include ../dist/include/nspr ../dist/include/xpcom
                    $(SYSTEM_DEVELOPER_DIR)/Headers/FlatCarbon
       * WebFeatures: ../dist/include ../dist/include/necko
                      ../dist/include/nspr ../dist/include/string
                      ../dist/include/xpcom
       * all others: ../dist/include
    + library search path:
       * Privacy, History, Downloads, WebFeatures: ../dist/lib

Comment 5

[Stuart Morgan]

Comment on attachment 292474 [details] [diff] [review]
Use aggregate targets to build the application; set bundle loader for preference panes

r+, but lets leave the -fshort-wchar until I finish deGeckoizing prefs, just in case.

Comment 6

[Mark Mentovai]

Checked in on the trunk, leaving -fshort-wchar.

Comment 7

[Mark Mentovai]

Reopened, this didn't play nice with Xcode 2.5's toolchain.

Comment 8

[Mark Mentovai]

Attachment: CmPreferencePane.framework

For reference, here's a patch that, in conjunction with attachment 292474 [details] [diff] [review], bases the preference panes on CmPreferencePane.framework.

Comment 9

[Mark Mentovai]

Attachment: Xcode 2.5-compatible fix

After poking this with a series of sticks of varying sizes, I found out that the problem is a bad interaction between -bundle_loader and -dead_strip in Xcode 2.5's ld.  As a workaround, we won't link prefpanes with -dead_strip.  This shouldn't be too big of a deal because there shouldn't really be any dead code in the prefpanes.

I'm also including a change that passes -executable_path to ld, because Xcode 2.5's toolchain wasn't reliably able to resolve dependencies of libxpcom_core.dylib.

The only changes between the earlier checkin and v2 are these lines added to mozilla/camino/config/PrefPane.xcconfig:

>+OTHER_LDFLAGS = >+-Wl,-executable_path,$(BUILD_DIR)/$(CONFIGURATION)/Camino.app/Contents/MacOS
>+
>+// -bundle_loader and -dead_strip interact poorly with one another in the Xcode
>+// 2.5 cctools, causing pref pane linking to fail with:
>+// ld: warning internal error: output_flush(offset = w, size = x) overlaps with flushed block(offset = y, size = z)
>+// This is not a problem in Xcode 3.0.  This can be removed once Xcode 3.0 is
>+// the minimum build requirement.
>+DEAD_CODE_STRIPPING = NO

v2 also includes OTHER_CFLAGS = -fshort-wchar, as requested by smorgan in comment 5.

Comment 10

[Mark Mentovai]

I tested this patch for a cycle on boxset (19:50 - 20:24), and it was green.

Comment 11

[Stuart Morgan]

Comment on attachment 292533 [details] [diff] [review]
Xcode 2.5-compatible fix

> I tested this patch for a cycle on boxset (19:50 - 20:24), and it was green.

Can't argue with that!

Comment 12

[Mark Mentovai]

Checked in.  Let's try this again!

Comment 13

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I'm nominating this for b1 just to make sure it stays on some sort of radar for branch landing while we let it bake.  Since it's blocking so many other bugs, I'm hoping we'll remember it, but you never know how we are ;)

Comment 14

[Mark Mentovai]

Attachment: 1.8 branch patch for Camino 1.6b1

I forgot to mention, and it applies to both the trunk patch and this patch: the Downloads preference pane needs its header search path to be set to:

../dist/include ../dist/include/dom ../dist/include/nspr ../dist/include/xpcom $(SYSTEM_DEVELOPER_DIR)/Headers/FlatCarbon

In my notes above, I neglected to list the FlatCarbon directory.

The one other change I needed in addition to the trunk project file update procedure was that the WebFeatures preference pane needed the header search path to include ../dist/include/cookie on the 1.8 branch.  This is because nsIPermission.h is in dist/include/cookie on the 1.8 branch, but has moved into dist/include/necko on the trunk.

Comment 15

[Mark Mentovai]

Tested static with Xcode 3.0 and shared with Xcode 2.5.  Checked in on MOZILLA_1_8_BRANCH pre-1.6b1.