Last Comment Bug 402892 - (gio) Switch from gnome-vfs to GIO
(gio)
: Switch from gnome-vfs to GIO
Status: RESOLVED FIXED
[needs GLib 2.16]
:
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: Trunk
: x86 Linux
: -- enhancement with 1 vote (vote)
: mozilla1.9.3a1
Assigned To: Jan Horak
:
: Nathan Froyd [:froydnj]
Mentors:
http://library.gnome.org/devel/gio/2....
: 455231 (view as bug list)
Depends on: 795169 1084193
Blocks: 466870 545320 829975
  Show dependency treegraph
 
Reported: 2007-11-07 11:23 PST by Hussam Al-Tayeb
Modified: 2014-10-19 17:31 PDT (History)
28 users (show)
benjamin: blocking1.9.2-
benjamin: wanted1.9.2-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
?


Attachments
Initial patch for GnomeVFS to GIO replacement (25.16 KB, patch)
2009-06-17 08:56 PDT, Jan Horak
no flags Details | Diff | Splinter Review
Patch v2: fixed headers and added missing idl file (30.22 KB, patch)
2009-06-18 01:08 PDT, Jan Horak
no flags Details | Diff | Splinter Review
Replace GnomeVFSService by GIOService in rest of the source code (3.77 KB, patch)
2009-06-18 03:05 PDT, Jan Horak
no flags Details | Diff | Splinter Review
Patch v3: removed unused parts (45.24 KB, patch)
2009-06-29 09:18 PDT, Jan Horak
ventnor.bugzilla: review+
timeless: review-
Details | Diff | Splinter Review
Patch v4: fixed leaks, using dlsym (46.69 KB, patch)
2009-08-10 06:20 PDT, Jan Horak
no flags Details | Diff | Splinter Review
Patch v4: keep GnomeVFS and g_content_type_from_mime_type workaround (46.36 KB, patch)
2009-08-20 02:42 PDT, Jan Horak
no flags Details | Diff | Splinter Review
Patch v6: do not enable gio by default, removed wince makefile [Checkin: Comment 56] (42.31 KB, patch)
2009-08-24 02:36 PDT, Jan Horak
roc: review+
Details | Diff | Splinter Review
system-headers bustage fix [Checkin: Comment 59] (594 bytes, patch)
2009-08-25 23:44 PDT, Reed Loden [:reed] (use needinfo?)
no flags Details | Diff | Splinter Review

Description Hussam Al-Tayeb 2007-11-07 11:23:19 PST
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a9pre) Gecko/2007110718 Firefox/3.0a9pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a9pre) Gecko/2007110718 Firefox/3.0a9pre

Currently firefox compiles against system gnome-vfs and it seems to work fine.
Under gnome 2.22, a migration to GVFS/gio is going to happen. I'm assuming distributions will ship GVFS instead of gnome-vfs.
How will firefox handle the swtch to GVFS?
A gvfs release is available here http://ftp.gnome.org/pub/GNOME/sources/gvfs/0.0/
http://ftp.gnome.org/pub/GNOME/sources/gio-standalone/0.1/


Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Comment 1 Elmar Ludwig 2007-11-09 03:45:19 PST
Is this supposed to be a bug report?

GVFS/gio don't have a stable API right now and there are discussions of merging gio into glib, so I think all of this is a bit early.

If you want to ask about the future plans regarding gnome-vfs, you should ask on the newsgroups or the support forums.
Comment 2 Hussam Al-Tayeb 2007-11-09 05:47:05 PST
Oh, sorry. I didn't mean to make it look like a question.
What I meant to say was. "Is this a realistic enhancement request or is it out of reach?"

But yes, GVFS/gio doesn't have a stable api yet. Maybe this should be deferred until it does in the next few months. Sorry for jumping the gun.
Comment 3 Mike Hommey [:glandium] 2008-02-08 17:46:11 PST
I guess, considering that firefox binaries as distributed by mozilla are supposed to have somewhat a broad support, that it would be better for a while to keep gnomevfs and have an additionnal module for gvfs/fio.
Comment 4 Reed Loden [:reed] (use needinfo?) 2008-02-08 17:51:03 PST
Yeah, it'll be a long while before we can even think about getting rid of gnomevfs.
Comment 5 Christopher Aillon (sabbatical, not receiving bugmail) 2008-02-10 12:57:25 PST
Note that mozilla.org is not shipping Linux binaries for release versions from here on out, only testing/nightly builds.
Comment 6 Matthias Versen [:Matti] 2008-09-14 14:11:46 PDT
*** Bug 455231 has been marked as a duplicate of this bug. ***
Comment 7 pbrobinson 2008-11-25 15:17:36 PST
I believe the gvfs/gio api is stable now. And most of gnome is now ported to it as of 2.24 with plans to deprecate gnomevfs now I believe.
Comment 8 Arpad Borsos [:Swatinem] 2009-01-06 05:10:57 PST
GIO is part of GLib since Version 2.16.
I'm not sure what Version of GLib Gecko requires, configure.in says version 1.2, which sounds quite broken ( http://mxr.mozilla.org/mozilla-central/source/configure.in#118 )
But GTK+ 2.10 has a dependency on GLib 2.12. Only GTK+ 2.14 requires a GLib version which includes GIO.
Comment 9 Tomeu Vizoso 2009-04-05 04:02:16 PDT
gnomevfs support in nsIconChannel is a bit hacky, see #486925 for an example of its consequences.

gvfs and gio should make much easier for non-gnome apps to use that functionality.
Comment 10 Arpad Borsos [:Swatinem] 2009-05-17 05:32:41 PDT
I have looked a little at nsIGnomeVFSService and users, namely nsGNOMERegistry, nsMIMEInfoUnix, nsLocalFileUnix and nsGNOMEShellService.

I believe a lot of this can be made significantly easier by using GIOs GAppInfo and GContentType. Maybe nsIGnomeVFSService can even go completely when switching to GIO.

This looks like a *lot* of work though. I may take a closer look at this at some time, I wanted to get deeper into GLib / GIO anyway.
Comment 11 Christopher Aillon (sabbatical, not receiving bugmail) 2009-05-17 15:40:19 PDT
Assigning to Jan, who has been working on this.
Comment 12 Michael Ventnor 2009-06-07 23:13:10 PDT
Jan, caillon, any news on this?
Comment 13 Jan Horak 2009-06-08 23:12:56 PDT
I'm still working on it.
Comment 14 Jan Horak 2009-06-17 08:56:03 PDT
Created attachment 383728 [details] [diff] [review]
Initial patch for GnomeVFS to GIO replacement

Following patch replaces GnomeVFS by GIO in following tasks:
- launch associated application for given extension, mime type or uri.
- set as default application for specified mime types and file extensions
Comment 15 Hussam Al-Tayeb 2009-06-17 10:56:50 PDT
patch doesn't add the nsIGIOService.idl file
Comment 16 Reed Loden [:reed] (use needinfo?) 2009-06-17 11:04:47 PDT
Comment on attachment 383728 [details] [diff] [review]
Initial patch for GnomeVFS to GIO replacement

>+ * The Original Code is the Mozilla GNOME integration code.
>+ *
>+ * This code is based on nsGnomeVFSService module.
>+ * Portions created by the Initial Developer are Copyright (C) 2009
>+ * the Initial Developer. All Rights Reserved.
>+ *
>+ * Contributor(s):
>+ *  Jan Horak <jhorak@redhat.com>

This is an invalid license header. Please make sure you use the exact text that the MPL boilerplate uses, as shown below:

 * The Original Code is __________________________________________.
 *
 * The Initial Developer of the Original Code is
 * ____________________________________________.
 * Portions created by the Initial Developer are Copyright (C) 2___
 * the Initial Developer. All Rights Reserved.
 *
 * Contributor(s):
Comment 17 Michael Ventnor 2009-06-17 17:46:17 PDT
You have to add an email address of the person you want review from if you want to get a review.

The problem is I'm not really sure who'd be suitable. bsmedberg? vlad? roc?
Comment 18 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-06-17 18:04:41 PDT
None of us are really familiar with this code. How about you, Michael, with me for an additional layer of super-review?
Comment 19 Michael Ventnor 2009-06-17 20:15:25 PDT
I suppose, but I won't be able to do it for a couple of weeks.
Comment 20 Arpad Borsos [:Swatinem] 2009-06-17 23:48:51 PDT
This is a straight port of nsIGnomeVFSService, good so far.
Last time I skimmed through the code, it looked quite feasible to get rid of the XPCOM-style interface completely, rather using the gio functions directly instead of wrapping them with an interface/class.

I don't believe this functionality is used through scripts, but if it is, it may be a better idea to think up a DOM interface that has implementations for all tier1 platforms.
Comment 21 Jan Horak 2009-06-18 01:08:41 PDT
Created attachment 383884 [details] [diff] [review]
Patch v2: fixed headers and added missing idl file

Thanks for comments, sorry for missing idl file. Headers fixed.
Comment 22 Jan Horak 2009-06-18 03:05:14 PDT
Created attachment 383890 [details] [diff] [review]
Replace GnomeVFSService by GIOService in rest of the source code

This patch replaces nsGnomeVfsService class by nsGIOService in Mozilla source code.
Comment 23 Mike Hommey [:glandium] 2009-06-18 11:25:22 PDT
(In reply to comment #20)
> This is a straight port of nsIGnomeVFSService, good so far.
> Last time I skimmed through the code, it looked quite feasible to get rid of
> the XPCOM-style interface completely, rather using the gio functions directly
> instead of wrapping them with an interface/class.

This is not a so good idea, especially for people that don't care about gnome and don't want anything related to gnome on their computer (think KDE users, for example). At least, it was true for gnomevfs. I guess it'd be the same with GIO. Keeping it as a XPCOM component allows it to be loaded when gnome libraries are there and not be loaded otherwise.

(In reply to comment #22)
> Created an attachment (id=383890) [details]
> Replace GnomeVFSService by GIOService in rest of the source code
> 
> This patch replaces nsGnomeVfsService class by nsGIOService in Mozilla source
> code.

Depending on how Mozilla wants to keep backwards compatibility, it might be worth keeping the GnomeVFS layer.
Comment 24 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-06-18 17:41:36 PDT
I agree we want to retain the separate dynamically loaded "GNOME module" that we have now. XPCOM generally sucks, but here it seems to be reasonably well-suited so I don't think we need to rip it out.

I'm not sure if we're ready to drop gnome-vfs support completely. With which GTK versions is GIO normally available?

Would it make sense to abstract out the common stuff from gnome-vfs and GIO into a common superinterface, so that it's easy for code to try GIO and gnome-vfs and use whichever one is available?

Thanks!!!
Comment 25 Hussam Al-Tayeb 2009-06-18 17:53:55 PDT
(In reply to comment #24)
> With which GTK versions is GIO normally available?
Gio has been part of glib since glib 2.16 released in March 2008.
Latest release is 2.20.3
Comment 26 Michael Ventnor 2009-06-18 20:14:38 PDT
(In reply to comment #24)
> I'm not sure if we're ready to drop gnome-vfs support completely.

I disagree. 3.6 is so far off that a library released in March 2008 seems quite reliable on a constantly-upgrading Linux desktop.
If 3.6 comes out one year after 3.5, then I think it'll be the same or longer than the period between GTK+ 2.10 and Firefox 3 that required it at a minimum.
Plus, IIRC, doesn't OLPC only ship with GIO and not gnome-vfs?
Less code to maintain and less interface complexity is always a good thing.
Comment 27 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-06-18 20:28:05 PDT
You disagree that I'm not sure? :-)

Karl may have an opinion on this.
Comment 28 Tomeu Vizoso 2009-06-19 00:48:20 PDT
(In reply to comment #26)
> (In reply to comment #24)
> Plus, IIRC, doesn't OLPC only ship with GIO and not gnome-vfs?

Yup, we had to branch the fedora packages in order to give up on this dependency.

http://koji.fedoraproject.org/koji/buildinfo?buildID=65584

> Less code to maintain and less interface complexity is always a good thing.

Not only that, but this also means that any app that embeds xulrunner as currently shipped by linux distros has to be a GNOME App, that meaning that they need to link and use deprecated libraries.

For example, the browser in Sugar has to do this workaround:

http://git.sugarlabs.org/projects/browse/repos/mainline/commits/c19e24ebe86bb1be38b9ff03fa2557248ebdf6b8

I lost several hours debugging this issue and I hope we can agree in that Gnome-vfs usage in mozilla is very inconvenient for embedders, specially in platforms with low resources.
Comment 29 Arpad Borsos [:Swatinem] 2009-06-19 05:44:00 PDT
Please correct me if I'm wrong:
The code currently handles file extension <-> mime type <-> application relations, sets up firefox as the default app for certain types and hands out other types to the corresponding default application.

For this special use case GIO itself is just a wrapper around xdg-mime, which is designed to be DE-independent.

What about using xdg-mime directly? That would make both parties happy as in removing dependency on a deprecated library and not introducing a new dependency on a library that kde people dislike, claiming to be too gnome-specific.
Comment 30 Michael Ventnor 2009-06-19 18:02:02 PDT
(In reply to comment #29)
> What about using xdg-mime directly? That would make both parties happy as in
> removing dependency on a deprecated library and not introducing a new
> dependency on a library that kde people dislike, claiming to be too
> gnome-specific.

For one, GIO is part of Glib which we already need. Secondly, xdg-mine is just a spec, not a library, so we'd end up needing to write our own parser for the mime type lookup which seems more work than necessary.
Comment 31 Mike Hommey [:glandium] 2009-06-19 22:57:51 PDT
(In reply to comment #30)
> For one, GIO is part of Glib which we already need.

Though, I'm not sure it will be enough by itself. i.e. will it return interesting things without gvfs?
Comment 32 Michael Ventnor 2009-06-28 03:14:46 PDT
OK, now I have time to look at this.

nsGnomeVFSService.cpp \
...
$(MOZ_GNOMEVFS_LIBS) \
etc...

Can't you remove all traces of GnomeVFS? I don't like the idea of us having code for both libraries in the tree especially if only one of them will be used. By the time mozilla-central becomes a release version I expect GIO to be more common than gnome-vfs on GNOME desktops. I think it's now or never to make this hard decision and I really want it to happen.

+NS_IMETHODIMP
+nsGIOMimeApp::GetCanOpenMultipleFiles(PRBool* aCanOpen)
+{
+  // GIO/GVFS does not provide such info
+  aCanOpen = PR_FALSE;
+  return NS_OK;
+}

+NS_IMETHODIMP
+nsGIOMimeApp::GetRequiresTerminal(PRBool* aRequires)
+{
+  // GIO/GVFS does not provide such info
+  *aRequires = PR_FALSE;
+  return NS_OK;
+}

...and any similar functions; are they used anywhere? I can't imagine why Mozilla needs to know these things, so just remove them if you can. Also remove these:

+  /* boolean keys */
+  const long APP_KEY_CAN_OPEN_MULTIPLE = 4;
+  const long APP_KEY_REQUIRES_TERMINAL = 5;

It seems like we can remove a lot of ugly code by making this switch so I'd like to make it happen. Thanks a lot for your work so far!
Comment 33 Jan Horak 2009-06-29 09:18:18 PDT
Created attachment 385767 [details] [diff] [review]
Patch v3: removed unused parts

Thanks for your comments. I've changed patch to replace GnomeVFS completely (excluding configure.in). I also removed unused parts and do some fixes to avoid leaking memory. Could you please do the review again?
Comment 34 Michael Ventnor 2009-06-29 19:37:37 PDT
Comment on attachment 385767 [details] [diff] [review]
Patch v3: removed unused parts

Looks good here. Thanks! roc now needs a look at this.
Comment 35 Karl Tomlinson (:karlt) 2009-06-29 20:42:18 PDT
These distribution releases seem to have glib-2.16 available
(perhaps through updates):

distro release release-date

Ubuntu    8.04 2008-04-24
Fedora    9    2008-05-13
openSUSE 11.0  2008-06-19
Debian    5.0  2009-02-15
Comment 36 pbrobinson 2009-07-30 02:51:31 PDT
Does the move to blocking1.9.2- and wanted1.9.2- mean its been dropped from 1.9.2 or just the beta. The next release of gnome will have no dependencies on gnome-vfs at all.
Comment 37 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-07-30 21:25:03 PDT
(In reply to comment #26)
> I disagree. 3.6 is so far off that a library released in March 2008 seems quite
> reliable on a constantly-upgrading Linux desktop.
> If 3.6 comes out one year after 3.5, then I think it'll be the same or longer
> than the period between GTK+ 2.10 and Firefox 3 that required it at a minimum.
> Plus, IIRC, doesn't OLPC only ship with GIO and not gnome-vfs?
> Less code to maintain and less interface complexity is always a good thing.

It looks like we're going to try to release 3.6 this year, seriously enough that we're nearly feature frozen. If we're successful, is it really OK for a release in November/December 2009 to depend on a library that was first released in March 2008, so that everyone who hasn't upgraded their distro within the last 18 months loses functionality? I'm not convinced.

The way I see it we have three options:
1) Put GIO in 1.9.2; people who haven't upgraded their distros will yell at us and say we hate freedom because we're not supporting them
2) Stick with GVFS for 1.9.2; people on the bleeding edge will yell at us and say we hate freedom because we're not supporting the latest and greatest
3) Support both; people will yell at us for being bloated

However, given that most distros are already on ridiculously short support cycles, we're probably already going to support most of them longer than the distro vendors themselves, so I vote for option #1.
Comment 38 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-07-30 21:34:38 PDT
However, our build machines are still on Centos5, so they can't build this. They'll have to be upgraded before we can land this.
Comment 39 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-07-30 21:42:24 PDT
+#if MOZ_GIO_VERSION >= 220
+  // For Glib >= 2.20 we can get commandline:
+  const char *cmd = g_app_info_get_commandline(mApp);

So if you build with Glib > 2.20, your build won't run on anything less than 2.20? That seems bad. Seems like we should do the dlsym dance here.

+nsGIOService::GetMimeTypeFromExtension(const nsACString &aExtension,
+                                             nsACString& aMimeType)

Please be consistent, I suggest nsACString&.

+  char *content_type = g_content_type_guess(fileExtToUse.get(),
+                                            NULL,
+                                            0,
+                                            &result_uncertain);
+  if (!content_type)
+    return NS_ERROR_FAILURE;
+
+  char *mime_type = g_content_type_get_mime_type(content_type);
+  if (!mime_type)
+    return NS_ERROR_FAILURE;

You leak content_type if mime_type is null.

+nsGIOService::GetAppForMimeType(const nsACString     &aMimeType,
+                                nsIGIOMimeApp** aApp)

Unnecessary whitespace.

+  if (app_info) {
+    nsGIOMimeApp *mozApp = new nsGIOMimeApp(app_info);
+    NS_ENSURE_TRUE(mozApp, NS_ERROR_OUT_OF_MEMORY);
+    NS_ADDREF(*aApp = mozApp);
+  } else {
+    return NS_ERROR_FAILURE;
+  }
+  g_free(content_type);

You leak content_type if app_info is null.

+  char *content_type =
+    g_content_type_from_mime_type(PromiseFlatCString(aMimeType).get());
+  if (!content_type)
+    return NS_ERROR_FAILURE;
+
+  char *desc = g_content_type_get_description(content_type);
+  if (!desc)
+    return NS_ERROR_FAILURE;

You leak content_type if desc is null.

+  if (file)
+    g_object_unref(file);
+  if (spec)
+    g_free(spec);

Are these functions not null-safe?

+nsGIOService::CreateAppFromCommand(nsACString const    &cmd,
+                                   nsACString const    &appName,

Unnecessary whitespace

+#if MOZ_GIO_VERSION >= 220
+      // For Glib >= 2.20 we can compare commandline:
+      if (strcmp(g_app_info_get_commandline(app_info_from_list),
+                 PromiseFlatCString(cmd).get()) == 0)

Again, it doesn't seem good to not run on glib < 2.20

Otherwise the patch looks good, modulo the issues about dependencies.

This would be a lot easier to take if we could at least support both GVFS and GIO in our source tree for an interim period.
Comment 40 Michael Ventnor 2009-07-30 23:30:20 PDT
(In reply to comment #39)
> This would be a lot easier to take if we could at least support both GVFS and
> GIO in our source tree for an interim period.

Oh dear, that's in direct contradiction to my review comment which said to completely strip gnome-vfs from the tree...
I still think having both in the tree is nonsensical, and if it has to miss 3.6 then as long as 3.6 really is a fast release I suppose everyone can wait just a little longer. I can :-)
Comment 41 timeless 2009-07-31 01:00:54 PDT
Comment on attachment 385767 [details] [diff] [review]
Patch v3: removed unused parts

diablo (maemo4.1) uses glib 2.12

We don't want to break our primary public embedding customer :)

fremantle (maemo5.0) will have 2.20, but for the time being, breaking our public embedding target is not acceptable.

There's nothing wrong with having configure able to try to support both, and dropping the ones it can't, and to runtime dynamically loading whichever ones work/are available.
Comment 42 Mark Finkle (:mfinkle) (use needinfo?) 2009-07-31 07:42:13 PDT
In addition to timeless' concern about glib versions on Maemo, I am interested
in unifying the desktop/maemo code we currently have in the tree. If you look
at the trunk nsLocalFileUnix::Reveal, you'll find some Maemo/Hildon code in
there as well:

http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsLocalFileUnix.cpp#1664

Same here (and several other places in the file):

http://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/unix/nsMIMEInfoUnix.cpp#87

I'm hoping that the new GIO support will "Just Work" on Maemo too. If not, I'd
like to move the #ifdef stuff into the service, instead of scattering it around
the tree.

Thoughts?
Comment 43 Christopher Aillon (sabbatical, not receiving bugmail) 2009-08-04 14:25:57 PDT
*** Bug 494163 has been marked as a duplicate of this bug. ***
Comment 44 Jan Horak 2009-08-10 06:20:11 PDT
Created attachment 393504 [details] [diff] [review]
Patch v4: fixed leaks, using dlsym

Thanks for the review. I've tried to fix leaks and also rewrote code dependent on Glib > 2.20 to use dlopen/dlsym. Can you look into it please?
Comment 45 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-08-10 18:13:53 PDT
This resolves all my comments, thanks! Except for the issue about removing GVFS support, I still think we can't do that yet :-(
Comment 46 Michael Ventnor 2009-08-10 19:26:16 PDT
Are we planning this for 3.6? If not, I don't think gnomevfs will be necessary.
Comment 47 Karl Tomlinson (:karlt) 2009-08-10 19:50:37 PDT
That might depend on what happens with Maemo.
IIUC, currently Fennec is aimed at Maemo 4.1, which does not have gio.

I don't know when Maemo 5 will be available, and it's not clear to me that 4.1 users will be able to upgrade.

What this means is that we'll at least need configure options to choose gnome-vfs and/or gio.

If we are going to keep a gnome-vfs module then we might as well make the choice between gio and gnome-vfs run-time selectable for older systems.  (IIUC the XPCOM API can be the same and only one module will be loaded, so there is [almost] no bloat.)

If the gio/gnome-vfs choice happens at run-time, then this probably could go into 1.9.2.

For the free desktop Linux distributions listed in comment 35, this is the projected end of security support for releases with glib < 2.16.

distro   release  end-of-life

Ubuntu   6.06 LTS 2009-06-01
Fedora   8        2009-12-25
openSUSE 10.3     2009-10-31
Debian   4.0      2010-02-15
Comment 48 pbrobinson 2009-08-11 00:52:35 PDT
> distro   release  end-of-life
> 
> Ubuntu   6.06 LTS 2009-06-01
> Fedora   8        2009-12-25
> openSUSE 10.3     2009-10-31
> Debian   4.0      2010-02-15

Fedora 8 was EOL on 2009-01-07. Fedora 9 was EOL on 2009-07-10 and it did has gio

http://fedoraproject.org/wiki/LifeCycle/EOL
Comment 49 Karl Tomlinson (:karlt) 2009-08-11 01:37:28 PDT
Thanks for correcting my one-month-after-two-subsequent-releases math which was 1 year out there, sorry.
Comment 50 Karl Tomlinson (:karlt) 2009-08-13 22:29:55 PDT
>+NS_IMETHODIMP
>+nsGIOMimeApp::SetAsDefaultForMimeType(nsACString const& aMimeType)
>+{
>+  char *content_type =
>+    g_content_type_from_mime_type(PromiseFlatCString(aMimeType).get());
>+  if (!content_type)
>+    return NS_ERROR_FAILURE;

>+NS_IMETHODIMP
>+nsGIOService::GetAppForMimeType(const nsACString& aMimeType,
>+                                nsIGIOMimeApp**   aApp)
>+{
>+  *aApp = nsnull;
>+
>+  char *content_type =
>+    g_content_type_from_mime_type(PromiseFlatCString(aMimeType).get());
>+  if (!content_type)
>+    return NS_ERROR_FAILURE;

>+NS_IMETHODIMP
>+nsGIOService::GetDescriptionForMimeType(const nsACString& aMimeType,
>+                                              nsACString& aDescription)
>+{
>+  char *content_type =
>+    g_content_type_from_mime_type(PromiseFlatCString(aMimeType).get());
>+  if (!content_type)
>+    return NS_ERROR_FAILURE;
>+

It looks like g_content_type_from_mime_type() is only available from GLib 2.18:
http://library.gnome.org/devel/gio/unstable/ix03.html

Should we require GLib 2.18 for this XPCOM module or is there a reasonable way of doing this that would work with 2.16?
Comment 51 Jan Horak 2009-08-20 02:42:07 PDT
Created attachment 395548 [details] [diff] [review]
Patch v4: keep GnomeVFS and g_content_type_from_mime_type workaround

Okay, I've put back GnomeVFS and did required changes to the source. I also wrote own version of g_content_type_from_mime_type function to be able to compile under 2.16. Could you be so kind and check the last patch?
Comment 52 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-08-23 17:28:40 PDT
Seems like the changes to build/wince/tools/Makefile should not be here.

Otherwise looks good ... except I'm not sure about defaulting to building with gio yet. It seems to me that if we build with gio then the gnome component will fail to load when gio is not available so the gnomevfs paths won't work either.
Comment 53 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-08-23 17:29:25 PDT
But I think this patch would be fine if the default was for gio to be disabled in configure.in.
Comment 54 Jan Horak 2009-08-24 02:36:11 PDT
Created attachment 396184 [details] [diff] [review]
Patch v6: do not enable gio by default, removed wince makefile
[Checkin: Comment 56]

Sorry for wince makefile diff, I've modified configure.in to not to enable gio by default. Thanks for comments.
Comment 55 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-08-24 18:26:57 PDT
Comment on attachment 396184 [details] [diff] [review]
Patch v6: do not enable gio by default, removed wince makefile
[Checkin: Comment 56]

Thanks, let's get this in!
Comment 56 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-08-25 18:42:00 PDT
http://hg.mozilla.org/mozilla-central/rev/2572312e17df
Thanks!!!
Comment 57 Joseph Felps 2009-08-25 22:35:37 PDT
I get this compile error when building with --enable-gio:

nsGIOService.o: In function `nsGIOService::CreateAppFromCommand(nsACString const&, nsACString const&, nsIGIOMimeApp**)':
nsGIOService.cpp:(.text+0x98): undefined reference to `g_app_info_get_all'
nsGIOService.cpp:(.text+0x147): undefined reference to `g_app_info_get_name'
nsGIOService.cpp:(.text+0x29b): undefined reference to `g_app_info_create_from_commandline'
nsGIOService.o: In function `nsGIOService::ShowURI(nsIURI*)':
nsGIOService.cpp:(.text+0x33b): undefined reference to `g_app_info_launch_default_for_uri'
nsGIOService.o: In function `nsGIOService::ShowURIForInput(nsACString const&)':
nsGIOService.cpp:(.text+0x3a0): undefined reference to `g_file_new_for_commandline_arg'
nsGIOService.cpp:(.text+0x3b3): undefined reference to `g_file_get_uri'
nsGIOService.cpp:(.text+0x3ce): undefined reference to `g_app_info_launch_default_for_uri'
nsGIOService.o: In function `nsGIOMimeApp::GetName(nsACString&)':
nsGIOService.cpp:(.text+0x429): undefined reference to `g_app_info_get_name'
nsGIOService.o: In function `nsGIOService::GetMimeTypeFromExtension(nsACString const&, nsACString&)':
nsGIOService.cpp:(.text+0x53c): undefined reference to `g_content_type_guess'
nsGIOService.cpp:(.text+0x54c): undefined reference to `g_content_type_get_mime_type'
nsGIOService.o: In function `nsGIOMimeApp::SetAsDefaultForFileExtensions(nsACString const&)':
nsGIOService.cpp:(.text+0x694): undefined reference to `g_app_info_set_as_default_for_extension'
nsGIOService.o: In function `nsGIOMimeApp::GetSupportedURISchemes(nsIUTF8StringEnumerator**)':
nsGIOService.cpp:(.text+0x75d): undefined reference to `g_vfs_get_default'
nsGIOService.cpp:(.text+0x76e): undefined reference to `g_vfs_get_supported_uri_schemes'
nsGIOService.o: In function `nsGIOMimeApp::Launch(nsACString const&)':
nsGIOService.cpp:(.text+0x91c): undefined reference to `g_app_info_launch_uris'
nsGIOService.o: In function `nsGIOMimeApp::GetExpectsURIs(int*)':
nsGIOService.cpp:(.text+0x98b): undefined reference to `g_app_info_supports_uris'
nsGIOService.o: In function `nsGIOMimeApp::GetId(nsACString&)':
nsGIOService.cpp:(.text+0x99f): undefined reference to `g_app_info_get_id'
nsGIOService.o: In function `get_content_type_from_mime_type(char const*)':
nsGIOService.cpp:(.text+0x9c3): undefined reference to `g_content_types_get_registered'
nsGIOService.cpp:(.text+0x9d4): undefined reference to `g_content_type_get_mime_type'
nsGIOService.o: In function `nsGIOService::GetDescriptionForMimeType(nsACString const&, nsACString&)':
nsGIOService.cpp:(.text+0xa72): undefined reference to `g_content_type_get_description'
nsGIOService.o: In function `nsGIOService::GetAppForMimeType(nsACString const&, nsIGIOMimeApp**)':
nsGIOService.cpp:(.text+0xaf9): undefined reference to `g_app_info_get_default_for_type'
nsGIOService.o: In function `nsGIOMimeApp::SetAsDefaultForMimeType(nsACString const&)':
nsGIOService.cpp:(.text+0xbad): undefined reference to `g_app_info_set_as_default_for_type'
/usr/bin/ld: libmozgnome.so: hidden symbol `g_app_info_get_name' isn't defined
/usr/bin/ld: final link failed: Nonrepresentable section on output
collect2: ld returned 1 exit status
make[4]: *** [libmozgnome.so] Error 1

System is x86_64 with glib 2.20.4.
Comment 58 Mike Hommey [:glandium] 2009-08-25 22:40:31 PDT
The GIO header file needs to be added to config/system-headers
Comment 59 Reed Loden [:reed] (use needinfo?) 2009-08-25 23:44:42 PDT
Created attachment 396676 [details] [diff] [review]
system-headers bustage fix
[Checkin: Comment 59]

Landed this as http://hg.mozilla.org/mozilla-central/rev/a1a1591eee2a
Comment 60 Mark Banner (:standard8, afk until Dec) 2009-08-26 00:18:52 PDT
(In reply to comment #59)
> Created an attachment (id=396676) [details]
> system-headers bustage fix
> 
> Landed this as http://hg.mozilla.org/mozilla-central/rev/a1a1591eee2a

I just pushed an additional fix to js/src/config to sync the system-headers files as unit test requires.
Comment 61 Reed Loden [:reed] (use needinfo?) 2009-08-26 00:27:58 PDT
(In reply to comment #60)
> I just pushed an additional fix to js/src/config to sync the system-headers
> files as unit test requires.

Somebody should put a big warning in config/system-headers about that. :(
Comment 62 pbrobinson 2010-01-05 04:30:59 PST
Can this become part of the "Lorentz" release?

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