299991 - XULRunner stub executable for mac

Status: RESOLVED FIXED

(Toolkit Graveyard :: XULRunner, defect, P1)

Description

[Benjamin Smedberg]

The XULRunner needs a stub executable which find the appropriate XULRunner(GRE)
installation based on version checks and then run that xulrunner installation.
It will use the standalone XPCOM glue to accomplish this task. It will set up
LD_LIBRARY_PATH so that the shell scripts that have traditionally been used are
no longer necessary.

On Mac and Windows this binary will be installed with every xulrunner application.
On *nix it will be installed in the path /usr/bin/xulrunner (the newest
XULRunner RPM should take precedence here, is that possible without causing
massive conflicts between concurrently installed XULRunner RPMs?)

Comment 1

[Wolfgang Rosenauer [:wolfiR]]

I see no really clean solution.
It would be possible to do some magic in the postinstall-scripts and using
symlinks to the newest xulrunner-bin binary.
The bad thing is then, that the link (or binary) does not belong to any RPM
package. I'll think about it some more minutes though.

Comment 2

[Todd A. Fisher]

Attachment: Mac App Bundle Starter Executable

This is a mac specific executable I'm using to test bug 300139 with.  One thing
I'm wondering is do we need this extra executable?  It's going to make
debugging a little bit more tricky for people,	also, all we're really doing
here is using this stub to pass the application.ini to the xulrunner-bin.  I'm
wondering if this can be done directly through the application bundle,
Info.plist?

Comment 3

[Benjamin Smedberg]

I don't understand the question. How is the application bundle going to be able
to find the proper XULRunner without a binary?

Comment 4

[Benjamin Smedberg]

Attachment: XULRunner stub executable for mac, rev. 1

The folks on freenode#macdev were extraordinarily helpful in describing how to
lie to an executable about what bundle it's in.

Comment 5

[Darin Fisher]

bsmedberg: can you provide a quick overview of how this patch works?

Comment 6

[Benjamin Smedberg]

Take a look at attachment 189049 [details]: that's the structure for a mac application
bundle I am hoping to follow: the xulrunner stub sits by itself in
Cotnents/MacOS and the xulapp contents are in Contents/Resources (I did this for
easy development/debugging, so that a developer could make Contents/Resources a
symlink to their xulapp development directory or even directly into
dist/xpi-stage/foo).

The stub itself is very simple, it just finds the path of the GRE (using the
glue functions) and it's own internal application.ini and then execvs to the XUL
framework xulrunner-bin.

The "trick" is when we pass argv[0] to xulrunner-bin: if you pass
/Library/Frameworks/XUL.framework/Versions/blah/xulrunner-bin as argv[0]
xulrunner-bin acts like an unbundled binary (no menus, no dock icon, etc).
Instead, we pass /Path/To/Simple.app/Contents/MacOS/xulrunner as argv[0], which
makes CFBundleGetMainBundle return the path to the correct appliction bundle and
all is happy, except now the xulrunner itself is confused: XRE_GetBinaryPath
uses CFBundleGetMainBundle which is now incorrect, so I am passing the xulrunner
directory as an evironment variable to xulrunner-bin in XRE_BINARY_PATH.

The change to configure.in was required because if you use the -l<lib> syntax
the lib is not added to LIBS_DEPS at
http://lxr.mozilla.org/mozilla/source/config/rules.mk#681, which means that
depend builds don't work correctly. Since there is never a chance that
xpcomglue/xpcomglue_s will be sharedlibs, this change should be meaningless
except to our dependency system.

Comment 7

[Darin Fisher]

Thanks for the details.  This sounds really good.  I'll try to review this shortly.

Comment 8

[Darin Fisher]

Comment on attachment 189052 [details] [diff] [review]
XULRunner stub executable for mac, rev. 1

>Index: toolkit/xre/nsAppRunner.cpp

> NS_IMETHODIMP
>+nsXULAppInfo::GetPlatformVersion(nsACString& aResult)
>+{
>+  aResult.AssignLiteral(TOOLKIT_EM_VERSION);
>+
>+  return NS_OK;
>+}

It looks like this is already checked in on the trunk.


>+#ifdef XP_MACOSX
>+static char *gBinaryPath;
>+#endif
...
>+#ifdef XP_MACOSX
>+  // The xulrunner stub executable tricks CFBundleGetMainBundle on
>+  // purpose into lying about the main bundle path. It will set
>+  // XRE_BINARY_PATH to inform us of our real location.
>+  gBinaryPath = getenv("XRE_BINARY_PATH");
>+
>+  if (gBinaryPath && !*gBinaryPath)
>+    gBinaryPath = nsnull;
>+
>+  printf("gBinaryPath = %s\n", gBinaryPath);
>+#endif

Can gBinaryPath be declared |const char*|?

Shouldn't this printf be removed?


>Index: xulrunner/stub/Makefile.in

This file needs a LICENSE header.


>+MODULE    = xpcom
>+PROGRAM   = xulrunner$(BIN_SUFFIX)

module = xpcom ???  are you sure?


>Index: xulrunner/stub/nsXULStubOSX.cpp

LICENSE header!


>+  if (NS_FAILED(rv)) {
>+    // XXXbsmedberg: Do something much smarter here: notify the
>+    // user/offer to download/?
>+    return 1;
>+  }

How about at least using fprintf(stderr,... so that the user
can at least see a message in the OSX console.


>+    CFURLCreateCopyAppendingPathComponent(kCFAllocatorDefault,
>+                                          absResourcesURL,
>+                                          CFSTR("application.ini"),
>+                                          false);

Ah, so now application.ini becomes the canonical name.	OK.


r=darin with these nits fixed

Comment 9

[Benjamin Smedberg]

Attachment: XULRunner stub executable for mac, rev. 1.1

Nits updated, requesting approval... this should have no affect on the current
toolkit apps, which do not use the glue.

Comment 10

[Mike Shaver (:shaver emeritus)]

Comment on attachment 190401 [details] [diff] [review]
XULRunner stub executable for mac, rev. 1.1

a=shaver

Comment 11

[Benjamin Smedberg]

Fixed on trunk for 1.8b4