961936 - Migrate XULRunner stub and install_app.py into the Gecko SDK

Status: RESOLVED WONTFIX

(Firefox Build System :: General, defect)

Description

[Alex Vincent [:WeirdAl]]

This should mostly be straight-forward.

Comment 1

[Alex Vincent [:WeirdAl]]

Attachment: work in progress, untested

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.

Comment 2

[Alex Vincent [:WeirdAl]]

Attachment: mvStub.diff

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

Comment 3

[Benjamin Smedberg]

If we remove --owner and --group we don't need --numeric-owner, right?

Comment 4

[Alex Vincent [:WeirdAl]]

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

[Benjamin Smedberg]

Comment on attachment 8367115 [details] [diff] [review]
mvStub.diff

Please remove the --numeric-owner flag, then I think this is ok.

Comment 6

[Alex Vincent [:WeirdAl]]

Attachment: patch for checkin

Comment 7

[Mike Kaply [:mkaply]]

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.

Comment 8

[Alex Vincent [:WeirdAl]]

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.

Comment 9

[Mike Hommey [:glandium]]

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

Comment 10

[Alex Vincent [:WeirdAl]]

Attachment: build.log as requested

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.

Comment 11

[Mike Hommey [:glandium]]

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.

Comment 12

[Benjamin Smedberg]

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.

Comment 13

[Alex Vincent [:WeirdAl]]

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.

Comment 14

[Alex Vincent [:WeirdAl]]

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

Comment 15

[Alex Vincent [:WeirdAl]]

> 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

Comment 16

[Alex Vincent [:WeirdAl]]

(that's after installing MacPorts's variant of gnutar.)

Comment 17

[Alex Vincent [:WeirdAl]]

Attachment: patch (almost there)

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.

Comment 18

[Benjamin Smedberg]

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?

Comment 19

[Alex Vincent [:WeirdAl]]

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

Comment 20

[Alex Vincent [:WeirdAl]]

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

Comment 21

[Alex Vincent [:WeirdAl]]

Attachment: patch updated to package_app, bug 1060059 changes

Comment 22

[Alex Vincent [:WeirdAl]]

Attachment: firefox-36.0a1.en-US.linux-x86_64.sdk-files.txt

Comment 23

[Alex Vincent [:WeirdAl]]

Attachment: post-package-app.stack

Comment 24

[Alex Vincent [:WeirdAl]]

Attachment: post-package-app.stack

Comment 25

[Alex Vincent [:WeirdAl]]

Attachment: firefox-36.0a1.en-US.mac-x86_64.sdk-files.txt

Comment 26

[Alex Vincent [:WeirdAl]]

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?

Comment 27

[Alex Vincent [:WeirdAl]]

Attachment: firefox-36.0a1.en-US.win32.sdk-files.txt

Well, at least everything works on Windows 7...

Comment 28

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

Why is this patch adding stuff to browser/? Can you avoid that?