Closed Bug 383085 Opened 13 years ago Closed 10 years ago

[meta] Eliminate Camino's use of xpfe

Categories

(Camino Graveyard :: General, defect)

PowerPC
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: alqahira, Unassigned)

References

Details

(Keywords: meta, Whiteboard: [not needed for 1.9])

Attachments

(3 files, 2 obsolete files)

We keep having talks about doing this, but everything remains pretty ephemeral. Now there's a bug; hopefully things become more concrete from here on out.

AFAIK, the two big things we use in xpfe are autocomplete and history; not sure what else there might be, other than a bunch of build/project changes.

We don't really use much of anything from autocomplete and just want to do it in Cocoa (bug 340611).

There's bug 351351 filed on mork->sqlite; there's also a bug I can't find at the moment where Håkan was doing some other incremental history improvements that might be relevant here.
(In reply to comment #0)
> there's also a bug I can't find at
> the moment where Håkan was doing some other incremental history improvements
> that might be relevant here.

If I'd re-read the autocomplete bug (bug 340611) before commenting, I'd have realized it was the one where Håkan was improving stuff last year :P
You can keep using autocomplete and history from xpfe even if the the rest is switched to toolkit, that's also what SeaMonkey is doing, and even Thunderbird still uses xpfe autocomplete at the moment.
See http://mxr.mozilla.org/mozilla/source/xpfe/components/Makefile.in - you'll notice that some things in there seem to be even built by Firefox and XULRunner.

Aparat from xpfe/components, you can abandon lots of old code by getting rid of xpfe startup code, chrome registry, etc. by just setting MOZ_XUL_APP=1 in Camino's confvars.sh
This patch removes the need to build xpfe/bootstrap from Camino and tries to fool tinderboxen to detect a non-zero, executable mozilla-bin so they stay green.
I can't test this without a Camino tinderbox though, so let's hope it works :)
Attachment #267616 - Flags: review?(mark)
Comment on attachment 267616 [details] [diff] [review]
eliminate xpfe/bootstrap from Camino build and fool tinderbox into detecting mozilla-bin (checked in)

>Index: mozilla/camino/Makefile.in
>===================================================================
>+clean clobber::
>+	rm -rf $(DIST)/$(APP_NAME).app

This needs to hardcode "Mozilla.ap" as well, of course.
Comment on attachment 267616 [details] [diff] [review]
eliminate xpfe/bootstrap from Camino build and fool tinderbox into detecting mozilla-bin (checked in)

r=me with Mozilla.app hardcoded in the clean/clobber target, if this disturbs the old tinderboxen, we'll fix it up when it happens.
Attachment #267616 - Flags: review?(mark) → review+
Comment on attachment 267616 [details] [diff] [review]
eliminate xpfe/bootstrap from Camino build and fool tinderbox into detecting mozilla-bin (checked in)

OK, checked in, let's see if it really works as we hope.
Attachment #267616 - Attachment description: eliminate xpfe/bootstrap from Camino build and fool tinderbox into detecting mozilla-bin → eliminate xpfe/bootstrap from Camino build and fool tinderbox into detecting mozilla-bin (checked in)
Depends on: 383833
Does Camino use mozilla/xpfe/components/console, updates, startup, alerts, or extensions?
Those directories are built ifndef MOZ_XUL_APP:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/xpfe/components/Makefile.in&rev=1.101&mark=92-100#85

Since Camino is the last app building without that flag set, we could remove those directories if Camino doesn't need them.

Bug 391938 is about removing xpfe/components/extensions.
Hmm. Since Camino does not support Firefox/SeaMonkey style extensions and thus recognizes neither the install.js nor the install.rdf method of installing extensions, I very much doubt that Camino uses nsExtensionManager.js at all.
(In reply to comment #8)
> Since Camino does not support Firefox/SeaMonkey style extensions

Camino does not support extensions *which require access to chrome*. There are a few extensions (Flashblock being the most obvious example) which Camino supports without problem.
> Camino does not support extensions *which require access to chrome*.

I am referring to the installation (install.js and install.rdf) process.

> (Flashblock being the most obvious example) which Camino supports
> without problem.

As you know Bob, I am an active Flashblock developer and also the Flashblock project contact person CC'ed to the Camino Flashblock bug, so I don't know why you are telling me this.
I'm wasn't telling you, just making sure it's noted here so that if someone less familiar with Camino does work in this bug they don't accidentally pull something out that we do need.
So could you please test this patch (preferably a clobber build) and see if anything breaks?
Attachment #286482 - Flags: review?(stuart.morgan)
Comment on attachment 286482 [details] [diff] [review]
stop building xpfe/components/console, updates, startup, alerts, extensions

> So could you please test this patch (preferably a clobber build) and see if
> anything breaks?

Yes: the build. Specifically, at the point where it failed, bootstrap referenced nsIAppStartup, which is in startup/.
Attachment #286482 - Flags: review?(stuart.morgan) → review-
As to why we are building anything in bootstrap, it appears that xpfe/bootstrap/appleevents is coming in through tier_toolkit.

I've asked this in various places, but apparently never this bug: why the big push to kill xpfe in Gecko 1.9? It's going to die a natural death in the Mozilla 2 source migration, and the expectation for Mozilla 2 seems to be that apps like Camino are going to have to be essentially ported to the new core (possibly with something like XULRunner solving all problems). Given that, trying to push Camino to toolkit and rip out xpfe at this point in the 1.9 cycle seems like a lot of effort that will be obsoleted relatively quickly by the M2 work.
With SeaMonkey having moved to toolkit, Camino is the last consumer of XPFE. CVS rm this from the tree will remove a lot of dead code. I noticed that in several recent bugs that patches were made against toolkit and then against the dead XPFE version of the same file because the person working on the bug did not know that the XPFE version had been abandoned, and spent unnecessary cycles getting the patch to work in XPFE, testing it, QA, etc.
Also, components/download-manager references nsIAlertsService from components/alerts
the toolkit download manager supports things like resume across sessions and lets you plug in custom UI quite easily, from what I hear, so Camino would probably be happier with that one anyways
Porting Camino to toolkit's download manager isn't in the scope of testing whether or not this patch builds; that should be a new bug blocking this one. I was just trying to anticipate the obvious question of whether a subset of this patch would work.

Continuing the chain of progressively smaller subsets, nsIExtensionManager.h (in extensions/) is included from files in /mozilla/xpinstall/
Removing just console and updates builds and launches. I haven't had time for any real testing though.
Depends on: 402294
Switching download turned out to be very easy (bug 402294). With that change, removing alert/ doesn't kill the build.
We may as well stop building the parts that we can easily remove.
Attachment #286482 - Attachment is obsolete: true
Attachment #317021 - Flags: review?(mark)
Stuart: Good idea - I think you should go all the way though and cvs remove the whole directories along with this :)
Smokey pointed out that we don't build xpinstall any more, so extensions can go as well.
Attachment #317021 - Attachment is obsolete: true
Attachment #317069 - Flags: review?(mark)
Attachment #317021 - Flags: review?(mark)
Attachment #317069 - Flags: review?(mark) → review+
Comment on attachment 317069 [details] [diff] [review]
stop building xpfe/components/{console,updates,alerts,extensions} [checked-in]

Requesting approval for 1.9; at this point this change should only affect Camino.
Attachment #317069 - Flags: approval1.9?
Comment on attachment 317069 [details] [diff] [review]
stop building xpfe/components/{console,updates,alerts,extensions} [checked-in]

a1.9=beltzner
Attachment #317069 - Flags: approval1.9? → approval1.9+
Comment on attachment 317069 [details] [diff] [review]
stop building xpfe/components/{console,updates,alerts,extensions} [checked-in]

Checking in xpfe/components/Makefile.in;
/cvsroot/mozilla/xpfe/components/Makefile.in,v  <--  Makefile.in
new revision: 1.105; previous revision: 1.104
done
Attachment #317069 - Attachment description: stop building xpfe/components/{console,updates,alerts,extensions} → stop building xpfe/components/{console,updates,alerts,extensions} [checked-in]
I cvs removed xpfe/components/{console,updates,alerts,extensions} with r/sr=Neil via IRC.
We also need to remove those directories (that are already gone now!) from toolkit-makefiles.sh
Attachment #319063 - Flags: review?
Attachment #319063 - Flags: review? → review?(gavin.sharp)
Comment on attachment 319063 [details] [diff] [review]
remove xpfe/components/{console,updates,alerts,extensions} from toolkit-makefiles [checked in]

Changes to *-makefiles have blanket-r bsmedberg.
Attachment #319063 - Flags: review?(gavin.sharp) → review+
Comment on attachment 319063 [details] [diff] [review]
remove xpfe/components/{console,updates,alerts,extensions} from toolkit-makefiles [checked in]

requesting a1.9 - just removing lines for now unused and removed directories from toolkit-makefiles.sh
Attachment #319063 - Flags: approval1.9?
Comment 27+ really should have been in another bug; if there ends up being yet more follow-up that has to happen to comment 27's landing, please do open a new bug for it.
Comment on attachment 319063 [details] [diff] [review]
remove xpfe/components/{console,updates,alerts,extensions} from toolkit-makefiles [checked in]

a1.9=beltzner
Attachment #319063 - Flags: approval1.9? → approval1.9+
Attachment #319063 - Attachment description: remove xpfe/components/{console,updates,alerts,extensions} from toolkit-makefiles → remove xpfe/components/{console,updates,alerts,extensions} from toolkit-makefiles [checked in]
All patches appear to have landed, resolving FIXED. Please reopen ASAP if more work needs to be done here.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
As clearly indicated in the summary, this is a meta bug, and most of its dependencies are still open, so this is by no means fixed. The fact that people added patches for individual aspects of it here rather than filing new dependencies as should have been done is unfortunate, but doesn't change that.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Cool, thanks.
Whiteboard: [meta bug with a+ patches landed]
Whiteboard: [meta bug with a+ patches landed] → [not needed for 1.9]
Status: REOPENED → NEW
Keywords: meta
Target Milestone: Camino2.0 → ---
Depends on: 545341
Work here is essentially done; there are no parts of xpfe that Camino's application/UI features depend on; thanks dan and hendy for taking us the rest of the way home.

There are, however, still parts of xpfe that are embedded in the build system (e.g., comment 13 and comment 14, at the very least) and thus are still required for the build system to work.  However, there are diminishing returns to untangling them on 1.9.0, so we won't, but hopefully people are no longer so terribly concerned with removing xpfe code on 1.9.0, anyway.

Closing this FIXED--but no-one should try to go cvs remove the rest of xpfe (or rdf/chrome); if you do, I will come after you with philor's shotgun and a volley of hot lead.
Status: NEW → RESOLVED
Closed: 12 years ago10 years ago
Resolution: --- → FIXED
(In reply to comment #36)
>hopefully people are no longer so
> terribly concerned with removing xpfe code on 1.9.0, anyway.

I don't think anyone has any interest of changing such things on a stable branch, even less one where Firefox is being EOLed ;-)

It's good to see you guys are making the jump off xpfe, I hope that will enable us all to work together on the same modern code bases in later 1.9.x platforms.
You need to log in before you can comment on or make changes to this bug.