383085 - [meta] Eliminate Camino's use of xpfe

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

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

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.

Comment 1

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

(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

Comment 2

[Robert Kaiser]

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

Comment 3

[Robert Kaiser]

Attachment: eliminate xpfe/bootstrap from Camino build and fool tinderbox into detecting mozilla-bin (checked in)

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 :)

Comment 4

[Robert Kaiser]

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 5

[Mark Mentovai]

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.

Comment 6

[Robert Kaiser]

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.

Comment 7

[Steffen Wilberg]

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.

Comment 8

[Philip Chee]

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.

Comment 9

[Stuart Morgan]

(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.

Comment 10

[Philip Chee]

> 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.

Comment 11

[Stuart Morgan]

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.

Comment 12

[Steffen Wilberg]

Attachment: 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?

Comment 13

[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/.

Comment 14

[Stuart Morgan]

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.

Comment 15

[Philip Chee]

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.

Comment 16

[Stuart Morgan]

Also, components/download-manager references nsIAlertsService from components/alerts

Comment 17

[Robert Kaiser]

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

Comment 18

[Stuart Morgan]

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/

Comment 19

[Stuart Morgan]

Removing just console and updates builds and launches. I haven't had time for any real testing though.

Comment 20

[Stuart Morgan]

Switching download turned out to be very easy (bug 402294). With that change, removing alert/ doesn't kill the build.

Comment 21

[Stuart Morgan]

Attachment: stop building xpfe/components/{console,updates,alert}

We may as well stop building the parts that we can easily remove.

Comment 22

[Robert Kaiser]

Stuart: Good idea - I think you should go all the way though and cvs remove the whole directories along with this :)

Comment 23

[Stuart Morgan]

Attachment: stop building xpfe/components/{console,updates,alerts,extensions} [checked-in]

Smokey pointed out that we don't build xpinstall any more, so extensions can go as well.

Comment 24

[Stuart Morgan]

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.

Comment 25

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 317069 [details] [diff] [review]
stop building xpfe/components/{console,updates,alerts,extensions} [checked-in]

a1.9=beltzner

Comment 26

[Stuart Morgan]

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

Comment 27

[Robert Kaiser]

I cvs removed xpfe/components/{console,updates,alerts,extensions} with r/sr=Neil via IRC.

Comment 28

[Robert Kaiser]

Attachment: remove xpfe/components/{console,updates,alerts,extensions} from toolkit-makefiles [checked in]

We also need to remove those directories (that are already gone now!) from toolkit-makefiles.sh

Comment 29

[:Gavin Sharp [email: [e-mail]]]

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.

Comment 30

[Robert Kaiser]

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

Comment 31

[Stuart Morgan]

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 32

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 319063 [details] [diff] [review]
remove xpfe/components/{console,updates,alerts,extensions} from toolkit-makefiles [checked in]

a1.9=beltzner

Comment 33

[Ben Turner (not reading bugmail, use the needinfo flag!)]

All patches appear to have landed, resolving FIXED. Please reopen ASAP if more work needs to be done here.

Comment 34

[Stuart Morgan]

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.

Comment 35

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Cool, thanks.

Comment 36

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

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.

Comment 37

[Robert Kaiser]

(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.