Last Comment Bug 761516 - Build webapprt/linux on any gtk2 platform including *bsd
: Build webapprt/linux on any gtk2 platform including *bsd
Status: VERIFIED FIXED
[qa!]
:
Product: Firefox Graveyard
Classification: Graveyard
Component: Web Apps (show other bugs)
: Trunk
: x86 OpenBSD
: P1 normal
: Firefox 16
Assigned To: Landry Breuil (:gaston)
: Jason Smith [:jsmith]
Mentors:
Depends on:
Blocks: 745018
  Show dependency treegraph
 
Reported: 2012-06-04 23:24 PDT by Landry Breuil (:gaston)
Modified: 2016-02-04 15:00 PST (History)
10 users (show)
jsmith: in‑moztrap+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Build linux/webapprt on all gtk platforms (1.94 KB, patch)
2012-06-05 04:29 PDT, Landry Breuil (:gaston)
jst: review+
Details | Diff | Review
Build linux/webapprt on all gtk platforms (1.94 KB, patch)
2012-06-11 03:12 PDT, Landry Breuil (:gaston)
felipc: review+
Details | Diff | Review
Build linux/webapprt on all gtk platforms (aurora patch) (1.26 KB, patch)
2012-06-11 03:14 PDT, Landry Breuil (:gaston)
felipc: review+
gavin.sharp: approval‑mozilla‑aurora+
Details | Diff | Review
Rename webapprt/linux to webapprt/gtk2 (881 bytes, patch)
2012-06-11 11:36 PDT, :Felipe Gomes (needinfo me!)
mcastelluccio: review+
gavin.sharp: approval‑mozilla‑aurora+
felipc: checkin+
Details | Diff | Review

Description Landry Breuil (:gaston) 2012-06-04 23:24:44 PDT
Since landing of #745018 , make package fails on OpenBSD (and probably any non-tier1 platforms) since MOZ_WEBAPP_RUNTIME defaults to 1 because toolkit is gtk, but webapprt/Makefile.in only cares about Win/Mac/Linux. So package tries to add non-built files.

See bottom of http://buildbot.rhaalovely.net/builders/mozilla-central-amd64/builds/413/steps/package/logs/stdio.

I have no friggin' idea what webapprt is not if it's supposed to run on unices different from Linux (a quick look at the code shows it should), so there are two options:
- only define MOZ_WEBAPP_RUNTIME on platforms that _actually_ support it
- enable/add the BSDs (and others ?) to webapps/Makefile.in

For #2, the following unportable code :
if (readlink("/proc/self/exe", curExePath, MAXPATHLEN) == -1)
should be fixed to use something better (glandium pointed me at BinaryPath::Get used by {b2g,browser}/app/nsBrowserApp.cpp, dunno if that's 100% equivalent)
Comment 1 Jason Smith [:jsmith] 2012-06-04 23:28:05 PDT
Marco - Can you take a look at this?
Comment 2 Mike Hommey [:glandium] 2012-06-04 23:40:47 PDT
This should stay under webapps, because the "linux" runtime should be enabled, and fixed for support of non-linux.
Comment 3 Landry Breuil (:gaston) 2012-06-04 23:45:15 PDT
Trying a fix here (ie build under linux if TOOLKIT=gtk2 + replace readlink by BinaryPath::Get). 

Since we're tied to gtk here, might aswell use glib functions g_dirname for GetDirFromPath() and GIO functions for CopyFile()..
Comment 4 Landry Breuil (:gaston) 2012-06-05 04:29:27 PDT
Created attachment 630131 [details] [diff] [review]
Build linux/webapprt on all gtk platforms

This patches fixes the issue for me. dist/./webapprt-stub gives an error dialog about missing webapp.ini, so at least it goes further than GetBinary::Path.

One could argue that putting the gtk code under a linux/ dir was a slightly wrong decision...
Comment 5 Marco Castelluccio [:marco] 2012-06-05 07:00:40 PDT
The first implementation was using GIO, but I later decided not to use it (as Firefox isn't built with GIO by default).
I was working on the Linux implementation, this is why I put the code under the linux/ dir. Indeed we should set MOZ_WEBAPPS_RUNTIME only for Linux GTK2 or, if the implementation is working on other *nix platforms (even if it wasn't supposed to work), use your solution.

(You have to install an application to completely test the binary, this is probably why webapp.ini was missing)
Comment 6 Landry Breuil (:gaston) 2012-06-05 10:52:58 PDT
I see no reason it wouldn't work, and atm it breaks builds. As the code landed before the mc->aurora uplift the same failures probably happen on aurora, so once we have something commited to m-c i'll nominate it for approval-aurora..
Comment 7 :Felipe Gomes (needinfo me!) 2012-06-05 11:27:39 PDT
It'd be good to actually verify that it works in other *nix, and if it does maybe we should rename webapprt/linux to webapprt/gtk2?
Comment 8 Landry Breuil (:gaston) 2012-06-05 12:18:52 PDT
(In reply to Felipe Gomes (:felipe) from comment #7)
> It'd be good to actually verify that it works in other *nix, and if it does
> maybe we should rename webapprt/linux to webapprt/gtk2?

It 'works' on openbsd in the sense that it doesnt blow up at startup. If you give me a procedure on how to actually test it (ie dl a "webapp" and run it ?) i can do some testing.
Comment 9 :Felipe Gomes (needinfo me!) 2012-06-05 12:34:37 PDT
Yes, install a webapp either from https://apps.mozillalabs.com/appdir/ or http://testmanifest.com

The app should show up in your system. Then launch it and see if it opens the right webpage in a standalone application
Comment 10 Jason Smith [:jsmith] 2012-06-05 12:39:01 PDT
Here's a test case to try specifically:

1. Go to apps.mozillalabs.com/appdir
2. Install Mozilla QA WebRT Tester
3. Launch Mozilla QA WebRT Tester
4. Try some of the functionality in the Mozilla QA WebRT Tester such as:
   a. Launch a pop-up
   b. View a swf file
5. Close the app
Comment 11 Landry Breuil (:gaston) 2012-06-06 00:26:22 PDT
Mhh.. so what ive done so far with a 16.0a1 build with that patch :
- go to https://apps.mozillalabs.com/appdir/ (note that the applist doesnt show up in webkit browsers, and in ffx 12 if i try to install an app it says 'network error' - there should be smth for firefoxes not supporting webapps,no ?)
- click on install mozilla qa webrt
- there's the 'do you want to install' prompt, i select install
- the throbber which replaced the 'install button' spins indefinitely

If i look at my profile, i have
/home/landry/.mozilla/firefox/gupt1o5c.xxx/webapps/
webapps.json
{50ea0339-fddf-4952-bcb6-3552a85b0f30}/
{50ea0339-fddf-4952-bcb6-3552a85b0f30\}/manifest.webapp                                                                                   <

No sign of webapp.ini, so i'm not sure the installation completed. And where should the app 'show up in my system' ? It creates a launcher .desktop file somewhere ? In a fd.o compliant menu ? somewhere in firefox menus ? Where is it supposed to show up on linux ?
Comment 12 Marco Castelluccio [:marco] 2012-06-06 10:44:44 PDT
Thank you for your testing.

The installation creates a directory under your home and a desktop entry file under ~/.local/share/applications. 
Their name's format is: https://mxr.mozilla.org/mozilla-central/source/browser/modules/WebappsInstaller.jsm#655

The application icon will be in the applications menu (it depends on the desktop environment you're using).
Comment 13 :Felipe Gomes (needinfo me!) 2012-06-06 12:10:45 PDT
Landry: yeah the installation should create a launcher as Marco said. If it's not there the installation is failing at some point, and there should be a useful error message in the Error Console
Comment 14 Johnny Stenback (:jst, jst@mozilla.com) 2012-06-09 00:17:19 PDT
Comment on attachment 630131 [details] [diff] [review]
Build linux/webapprt on all gtk platforms

r=jst. If this doesn't work, it seems we should file a bug to put the /proc/self/exe code that this removes into mozilla::BinaryPath::Get(), and run that on linux systems instead of the guessing game that we do there now.
Comment 15 Landry Breuil (:gaston) 2012-06-11 03:07:08 PDT
Att #630131 didnt apply since Bug 749930  landed, posting another patch in a sec.. and a patch against aurora too.

I've also successfully tested webapp rt tester and marble run apps.
Comment 16 Landry Breuil (:gaston) 2012-06-11 03:12:50 PDT
Created attachment 631842 [details] [diff] [review]
Build linux/webapprt on all gtk platforms
Comment 17 Landry Breuil (:gaston) 2012-06-11 03:14:54 PDT
Created attachment 631843 [details] [diff] [review]
Build linux/webapprt on all gtk platforms (aurora patch)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 

745018

User impact if declined: 

Build failure on non-linux gtk2 platforms

Testing completed (on m-c, etc.): 

Yes, in this bug

Risk to taking this patch (and alternatives if risky): 

none ?

String or UUID changes made by this patch: 

none
Comment 18 Marco Castelluccio [:marco] 2012-06-11 03:32:44 PDT
This could also fix bug 763192, as on Ubuntu there are problems with /proc/self/exe on some kernels (https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1007089)
Comment 19 :Felipe Gomes (needinfo me!) 2012-06-11 09:14:57 PDT
Comment on attachment 631842 [details] [diff] [review]
Build linux/webapprt on all gtk platforms

Simple line number update, it's not necessary to re-request review on these cases
Comment 20 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-06-11 09:41:47 PDT
Let's get this landed on mozilla-central before approving for Aurora.
Comment 21 :Felipe Gomes (needinfo me!) 2012-06-11 11:36:30 PDT
Created attachment 631963 [details] [diff] [review]
Rename webapprt/linux to webapprt/gtk2

Marco, is changing webapprt/linux to webapprt/gtk2 something that we also want to change? Should we do this together with this bug or do you want to wait to check if there will be more work involved to support gtk2 on all *nix?
Comment 22 :Felipe Gomes (needinfo me!) 2012-06-11 11:37:43 PDT
Comment on attachment 631843 [details] [diff] [review]
Build linux/webapprt on all gtk platforms (aurora patch)

(I will re-request aurora approval after this sticks in mozilla-central)
Comment 23 Landry Breuil (:gaston) 2012-06-11 11:53:08 PDT
oops, i've already pushed att 631842 to m-i before seeing your patch..
http://hg.mozilla.org/integration/mozilla-inbound/rev/5c5336ccf186
Comment 24 Marco Castelluccio [:marco] 2012-06-11 12:04:54 PDT
Comment on attachment 631963 [details] [diff] [review]
Rename webapprt/linux to webapprt/gtk2

Yes, I think we can do this now. After all, it's just a name change :)
Comment 25 Graeme McCutcheon [:graememcc] 2012-06-12 03:05:55 PDT
https://hg.mozilla.org/mozilla-central/rev/5c5336ccf186
Comment 26 Jason Smith [:jsmith] 2012-06-12 06:46:41 PDT
Marco - Can you verify this fix on mozilla central or inbound?
Comment 27 Landry Breuil (:gaston) 2012-06-12 11:19:46 PDT
Shouldn't this bug stay open for the linux->gtk2 rename and the pending aurora approval ?
Comment 28 Jason Smith [:jsmith] 2012-06-12 11:24:16 PDT
(In reply to Landry Breuil (:gaston) from comment #27)
> Shouldn't this bug stay open for the linux->gtk2 rename and the pending
> aurora approval ?

Yes for the rename. For the aurora approval, set status-firefox15 flag to fixed when the aurora patch lands, but the bug is not reopened for that.
Comment 29 :Felipe Gomes (needinfo me!) 2012-06-12 13:50:29 PDT
Comment on attachment 631963 [details] [diff] [review]
Rename webapprt/linux to webapprt/gtk2

Landed the rename on inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/d42e98ca091a

we can now close the bug when it gets merged to m-c, and then request aurora approval
Comment 30 Ed Morley [:emorley] 2012-06-13 05:59:47 PDT
https://hg.mozilla.org/mozilla-central/rev/d42e98ca091a
Comment 31 Landry Breuil (:gaston) 2012-06-13 07:09:20 PDT
I suppose that both commits have to be backported to aurora, no ?
Comment 32 Jason Smith [:jsmith] 2012-06-13 08:58:46 PDT
(In reply to Landry Breuil (:gaston) from comment #31)
> I suppose that both commits have to be backported to aurora, no ?

Yes. Please nominate for Aurora.
Comment 33 Landry Breuil (:gaston) 2012-06-13 11:51:19 PDT
Comment on attachment 631843 [details] [diff] [review]
Build linux/webapprt on all gtk platforms (aurora patch)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 

745018

User impact if declined: 

Build failure on non-linux gtk2 platforms

Testing completed (on m-c, etc.): 

Yes, in this bug

Risk to taking this patch (and alternatives if risky): 

none ?

String or UUID changes made by this patch: 

none
Comment 34 Landry Breuil (:gaston) 2012-06-13 11:52:21 PDT
Comment on attachment 631963 [details] [diff] [review]
Rename webapprt/linux to webapprt/gtk2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 

745018

User impact if declined: 

Build failure on non-linux gtk2 platforms

Testing completed (on m-c, etc.): 

Yes, in this bug

Risk to taking this patch (and alternatives if risky): 

none ?

String or UUID changes made by this patch: 

non
Comment 36 Marco Castelluccio [:marco] 2012-06-22 08:52:01 PDT
I can't set *bsd as environment for a test case with MozTrap.
Comment 37 Landry Breuil (:gaston) 2012-06-22 09:03:38 PDT
What's needed to run such a test? If you give me the steps i can try
Comment 38 Marco Castelluccio [:marco] 2012-06-22 09:18:57 PDT
MozTrap is the mozilla tool to handle manual test cases, we should simply add *bsd as a platform where we should do webapps related tests.

I'd also need to verify the fix on *bsd, could you send me a bsd build?
Comment 39 Landry Breuil (:gaston) 2012-06-22 09:26:23 PDT
You can grab my builds at http://buildbot.rhaalovely.net/builds/ but you'll need to install openbsd -current to try it
Comment 40 Jason Smith [:jsmith] 2012-06-22 09:28:53 PDT
(In reply to Marco Castelluccio from comment #36)
> I can't set *bsd as environment for a test case with MozTrap.

Took care of adding it to the list of environments and linked it to the existing Linux test cases. If you search for "apps" and "Open BSD," then you should see the list of test cases for testing this feature so far under Open BSD. We should probably do a quick spot check to make sure that the web runtime works on Open BSD.
Comment 41 Jason Smith [:jsmith] 2012-06-29 13:18:48 PDT
Landry - Are you able to install web apps on the latest nightly and launch them on Open BSD?
Comment 42 Landry Breuil (:gaston) 2012-06-29 13:33:54 PDT
It worked 3 weeks ago.. do you think another change broke it in the meantime ? What do you want me to test in particular ?
Comment 43 Jason Smith [:jsmith] 2012-06-29 13:34:55 PDT
(In reply to Landry Breuil (:gaston) from comment #42)
> It worked 3 weeks ago.. do you think another change broke it in the meantime
> ? What do you want me to test in particular ?

In that case, I'll mark this as verified. Was looking to know if the patch works on a release build.

Note You need to log in before you can comment on or make changes to this bug.