306689 - XULRunner stub executable for win/linux

Status: RESOLVED FIXED

(Toolkit Graveyard :: XULRunner, defect, P1)

Description

[Benjamin Smedberg]

Parallels bug 299991, for linux/win.

Comment 1

[Benjamin Smedberg]

Attachment: XULRunner stub for linux/mac, rev. 1

Comment 2

[Robert Strong (they/them - no direct email)]

Sorry but I've been traveling and won't have time to review this properly for
another day.

+    var xulrunnerBinary = getDirectoryKey("XCurProcD");
+    xulrunnerBinary.append("xulrunner-stub@BIN_SUFFIX@");
+
+    xulrunnerBinary.copyTo(aDirectory, appName + "@BIN_SUFFIX@");
When using the appName to create the directory you change it to lowercase -
shouldn't the stub's filename be lowercase as well?

+  BOOL ok = WinLaunchChild(xulBin, argc + 1, argv2);
+
+  free (argv2);
+
+  return !ok;
Returning !ok seems wrong here.

+#else
+  execv(xulBin, argv2);
+
+  return 1;
Why not the result from execv?

Comment 3

[Benjamin Smedberg]

> shouldn't the stub's filename be lowercase as well?
yes

> +  BOOL ok = WinLaunchChild(xulBin, argc + 1, argv2);
> +
> +  free (argv2);
> +
> +  return !ok;
> Returning !ok seems wrong here.

Why? main() traditionally returns "0" for success and error codes for failure.

> Why not the result from execv?

If execv returns we already have a failure condition.

Comment 4

[Robert Strong (they/them - no direct email)]

Comment on attachment 199953 [details] [diff] [review]
XULRunner stub for linux/mac, rev. 1

Looks good - only nit I have is making the copy of the stub's name lowercase.

Comment 5

[Benjamin Smedberg]

Fixed, trunk and branch.

Comment 6

[Brian King [:kinger]]

Output from launch attempt on Linux. Trunk updated today.

linbrian:/home/mozilla/src/trunk/mozilla/xulrunner-opt-static/dist# bin/xulrunner-bin -install-app /mnt/hda2/mozilla/extensions/xulrunner/cz-xulrunner.xpi
bin/xulrunner-bin: error while loading shared libraries: libxul.so: cannot openshared object file: No such file or directory

Comment 7

[Benjamin Smedberg]

Brian, you still need to use the "xulrunner" script on linux when running xulrunner directly (-install-app for example).

Comment 8

[Brian King [:kinger]]

It's working now.