Migrate XULRunner stub and install_app.py into the Gecko SDK

RESOLVED WONTFIX

Status

defect
RESOLVED WONTFIX
6 years ago
8 months ago

People

(Reporter: WeirdAl, Assigned: WeirdAl)

Tracking

Trunk
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 6 obsolete attachments)

This should mostly be straight-forward.
Posted patch work in progress, untested (obsolete) — Splinter Review
This patch folds in attachment 8360177 [details] [diff] [review] from bug 923979.  The patch compiles on Linux; I have not tried the other platforms yet.
Component: General → Build Config
Posted patch mvStub.diff (obsolete) — Splinter Review
This patch has been tested at a basic level on Windows, Linux & Mac.  However, it's not perfect:  XULRunner builds invoking make sdk break for reasons I can't understand.  Something about the packager.mk changes caused that.

Also, in packager.mk, I removed references to --owner=0 --group=0.  Apparently on my Macbook, the tar command no longer supports those.

I tested first on Windows, fixed bustages there, then tested on Linux, fixed bustages there, finally on Mac and fixed bustages there - so although it's very unlikely, this patch might not fully work on Linux or Windows.  I'd love it if someone could give this a try run with |make sdk|.
Attachment #8362822 - Attachment is obsolete: true
Attachment #8367115 - Flags: review?(benjamin)
If we remove --owner and --group we don't need --numeric-owner, right?
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #3)
> If we remove --owner and --group we don't need --numeric-owner, right?

If you're asking me, I don't know anything about these arguments at all.
Comment on attachment 8367115 [details] [diff] [review]
mvStub.diff

Please remove the --numeric-owner flag, then I think this is ok.
Attachment #8367115 - Flags: review?(benjamin) → review+
Posted patch patch for checkin (obsolete) — Splinter Review
Attachment #8367115 - Attachment is obsolete: true
Attachment #8367467 - Flags: review+
Keywords: checkin-needed
We also need to bring over the redit tool as part of the "Firefox Runtime" or whatever we are calling this:

http://mxr.mozilla.org/mozilla-central/source/xulrunner/tools/redit/

It's used on Windows to change the icon of the xulrunner-stub.
helpwanted, glandium:  As I said in comment 2, I inadvertently broke |make sdk| for XULRunner with the changes in the patch.  Although we're trying to EOL the XULRunner SDK, bsmedberg says releng has yet "to build the SDK from FF this release".  I don't know how to fix the build bustage; Benjamin suggested you could.

Also, removing the checkin-needed keyword for redit.  mkaply should have a new bug filed for that soon.
Flags: needinfo?(mh+mozilla)
Keywords: checkin-needed
(In reply to Alex Vincent [:WeirdAl] from comment #2)
> Also, in packager.mk, I removed references to --owner=0 --group=0. 
> Apparently on my Macbook, the tar command no longer supports those.

I'm sure they were never supported by OSX tar. You need to use GNU tar. I'm not too thrilled about removing those flags.

(In reply to Alex Vincent [:WeirdAl] from comment #8)
> helpwanted, glandium:  As I said in comment 2, I inadvertently broke |make
> sdk| for XULRunner with the changes in the patch.

Without a build log, I can't tell anything.
Flags: needinfo?(mh+mozilla)
Posted file build.log as requested (obsolete) —
I'll figure out about getting gnu tar on my box later.  Here's the build log, including the mozconfig, ./mach build, and make -C (objdir) sdk.
Flags: needinfo?(mh+mozilla)
You're entering an infinite loop because in
  $(MAKE) $(MOZ_PKG_MANIFEST)

$(MOZ_PKG_MANIFEST) is empty for xulrunner. So that actually does $(MAKE). That being said, that hunk is useless, as the dependency is already doing what you're changing it for. I /guess/ you intended to add STAGE_SDK=1 but that just feels wrong. Why not just copy the files in the make-sdk recipe instead of adding them to the manifest? Relatedly, I'm not sure install_app.py has its place in the sdk.

Finally, copying the stub is really not something i recommend, because that just makes it stay outdated each time browser/app/nsBrowserApp.cpp is changed, which is one of the many reasons we want to get rid of xulrunner.
Flags: needinfo?(mh+mozilla)
I think we all agree that it would be better not to have the forked code for nsBrowserApp.cpp and nsXULRunnerApp.cpp. In the short term I don't think this is very easy to fix and so I'm happy with having both of them.

install_app.py should live in the SDK, although it should maybe have a name like "package_app.py" since what it really does is put together a runtime and an app into a single bundle. It's not meant to be run by end users installing something.
Per #xulrunner discussion, moving redit doesn't seem to be an immediate goal:  

[08:47]	mkaply	Actually, bsmedberg said not to worry about it. because the xulrunner directory isn't going away

So the only thing absolutely preventing the latest patch on this bug from landing is the aforementioned build bustage on XR.  Obviously renaming install_app to package_app is not a problem.
Ladies and gents, my apologies for not working on this further.  I've been under a boatload of stress lately, including an urgent family issue and a wisdom tooth extraction.  I do want to get back to this and get it fixed, but if someone else has time, feel free to take this.

(I leave way too many things started and not finished.)
> I'll figure out about getting gnu tar on my box later.

Note for self:  add this line to mozconfig, and the build will use gnutar:
TAR=/opt/local/bin/gnutar
(that's after installing MacPorts's variant of gnutar.)
Posted patch patch (almost there) (obsolete) — Splinter Review
So close, and yet...

make sdk will work, but it generates an incomplete SDK.  In particular, files like firefox-stub, omni.ja, xpcshell, platform.ini, plugin-container, etc. are missing.
Attachment #8367467 - Attachment is obsolete: true
Attachment #8370565 - Attachment is obsolete: true
What OS are you testing with? I'm not sure I understand why omni.ja would be in the SDK, since it's already part of the Firefox install. I figured that the SDK would only contain files that weren't in the normal Firefox runtime package, like .idl, headers, import libraries, and whatnot.

Anyway, the big difference between "normal" Firefox packaging and "normal" XULRunner packaging is that Firefox packaging has a manifest and XULRunner doesn't. Perhaps that's the problem you're facing here?
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #18)
> What OS are you testing with?
When I wrote the latest patch, that was with MacOS 10.10.1 Yosemite.  I'm retesting it now with Fedora 20, and soon with Windows 7.

> I'm not sure I understand why omni.ja would be
> in the SDK, since it's already part of the Firefox install. I figured that
> the SDK would only contain files that weren't in the normal Firefox runtime
> package, like .idl, headers, import libraries, and whatnot.

It appears to, yes, with or without any patch from this bug.  But the stub and package_app script (I forgot to rename from install_app) still aren't showing up.
(In reply to Alex Vincent [:WeirdAl] from comment #19)
> But the stub
> and package_app script (I forgot to rename from install_app) still aren't
> showing up.

Oh, yes, they are... (eyeroll)
Posted file post-package-app.stack (obsolete) —
Attachment #8528017 - Attachment is obsolete: true
Alexanders-MacBook-Pro:firefox-sdk ajvincent$ find . -name xpcshell
./sdk/bin/xpcshell
Alexanders-MacBook-Pro:firefox-sdk ajvincent$ find . -name firefox-stub
Alexanders-MacBook-Pro:firefox-sdk ajvincent$ 

On my Linux box, I do find the firefox-stub in ./bin/firefox-stub, and similarly for package_app.py, but not on the Mac build.  Build config help, please?
Well, at least everything works on Windows 7...
Why is this patch adding stuff to browser/? Can you avoid that?
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → WONTFIX
Component: Build Config → General
Product: Firefox → Firefox Build System
You need to log in before you can comment on or make changes to this bug.