Migrate XULRunner stub and install_app.py into the Gecko SDK

RESOLVED WONTFIX

Status

defect
RESOLVED WONTFIX
5 years ago
6 months ago

People

(Reporter: WeirdAl, Assigned: WeirdAl)

Tracking

Trunk
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 6 obsolete attachments)

Assignee

Description

5 years ago
This should mostly be straight-forward.
Assignee

Comment 1

5 years ago
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
Assignee

Comment 2

5 years ago
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)

Comment 3

5 years ago
If we remove --owner and --group we don't need --numeric-owner, right?
Assignee

Comment 4

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

5 years ago
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+
Assignee

Comment 6

5 years ago
Posted patch patch for checkin (obsolete) — Splinter Review
Attachment #8367115 - Attachment is obsolete: true
Attachment #8367467 - Flags: review+
Assignee

Updated

5 years ago
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.
Assignee

Comment 8

5 years ago
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)
Assignee

Comment 10

5 years ago
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.
Assignee

Comment 13

5 years ago
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.
Assignee

Comment 14

5 years ago
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.)
Assignee

Comment 15

5 years ago
> 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
Assignee

Comment 16

5 years ago
(that's after installing MacPorts's variant of gnutar.)
Assignee

Comment 17

5 years ago
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?
Assignee

Comment 19

5 years ago
(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.
Assignee

Comment 20

5 years ago
(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)
Assignee

Comment 23

5 years ago
Posted file post-package-app.stack (obsolete) —
Assignee

Comment 24

5 years ago
Attachment #8528017 - Attachment is obsolete: true
Assignee

Comment 26

5 years ago
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?
Assignee

Comment 27

5 years ago
Well, at least everything works on Windows 7...
Why is this patch adding stuff to browser/? Can you avoid that?
Assignee

Updated

a year ago
Status: NEW → RESOLVED
Last Resolved: a year ago
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.