After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 500922 - Test Patch to get rid of libgnome[ui]/libbonobo[ui]
Test Patch to get rid of libgnome[ui]/libbonobo[ui]
Status: RESOLVED FIXED
Product: gedit
Classification: Applications
Component: general
git master
Other All
: Normal enhancement
: ---
Assigned To: Gedit maintainers
Gedit maintainers
Depends on:
Blocks:
 
 
Reported: 2007-12-01 20:46 UTC by Mikael Hermansson
Modified: 2008-08-13 17:18 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
remove gnomelibs dependies testpatch (8.54 KB, patch)
2007-12-01 20:50 UTC, Mikael Hermansson
none Details | Review
remove gnomelibs testpatch (8.33 KB, patch)
2007-12-01 20:57 UTC, Mikael Hermansson
none Details | Review
Cleanup patch (15.57 KB, patch)
2007-12-02 18:10 UTC, Mikael Hermansson
none Details | Review
Kills gnome_help call (2.87 KB, patch)
2007-12-02 18:21 UTC, Mikael Hermansson
none Details | Review
Get rid of gnome_uitl calls (967 bytes, patch)
2007-12-02 18:34 UTC, Mikael Hermansson
none Details | Review
New patch (13.20 KB, patch)
2007-12-09 17:04 UTC, Mikael Hermansson
none Details | Review
New patch fixing gnome_vfs_show_uri (14.54 KB, text/plain)
2007-12-09 19:36 UTC, Mikael Hermansson
  Details
add g_option_context_parse if libgnome deprecated (14.59 KB, patch)
2007-12-10 01:14 UTC, Mikael Hermansson
none Details | Review
Small fix to timeplugin (16.86 KB, patch)
2007-12-16 17:21 UTC, Mikael Hermansson
none Details | Review
This patch is in sync with SVN version of gedit: (11.73 KB, patch)
2007-12-26 14:32 UTC, Mikael Hermansson
none Details | Review
New patch that works with SVN trunk (11.49 KB, patch)
2008-02-04 21:50 UTC, Mikael Hermansson
none Details | Review

Description Mikael Hermansson 2007-12-01 20:46:48 UTC
The below patch just removes the remaining dependies of libgnome and bonobo and popt.

To compile wihout libgnome dependies you MUST configure --enable-deprecated-gnome=no 


There is still some issues with this patch:

1. No support for a gnome_help_*  right know it just g_spawn gnome-help and loads the international version of the help file. This means no localized help document will automatically show in the helpbrower. 

2. No session support. Dont know any replacement but heard something about libegg has something...

3. gnome_icon_lookup is just dropped replacement will probadly be gio API



3. gnome_
Comment 1 Mikael Hermansson 2007-12-01 20:50:49 UTC
Created attachment 100001 [details] [review]
remove gnomelibs dependies testpatch
Comment 2 Mikael Hermansson 2007-12-01 20:57:41 UTC
Created attachment 100003 [details] [review]
remove gnomelibs testpatch
Comment 3 Paolo Maggi 2007-12-02 11:40:53 UTC
Hi Mikael, 
thanks for the patch.

Could you please split the patch in smaller patches with more focused goals?

For example, it would be nice to have a  patch to remove all the calls to libgnome[ui] functions (except gnome-program). Another one to replace gnome-session with the code in libegg. Another one that copies the code of gnome-help in a gedit-help-utils file (or something like that). Etc. 

The goal is to start cleaning up gedit removing all the unneeded dependencies in an incremental way without losing features.

Have you measured the gain in memory consumption and/or improved startup time of your patch?

Comment 4 Mikael Hermansson 2007-12-02 18:10:28 UTC
Created attachment 100055 [details] [review]
Cleanup patch

Here is a cleaner patch for gedit --without-libgnome option. 

Default is to compile WITH libgnome dependies. If user not want to compile without gnome libs he must pass --without-gnome. It should just configure/compile fine to if user has no libgnome/ui installed.

Updates:

1. gnome_help replaced with evince based code. Totally replaces gnome_help (no ifdefs here)!

2. Some calls to gnome has been replaced with glib calls that works exacly the same. 

3. All plugins should now compile with/without gnomelibs.

I will soon post splitted patches. with the code that is not "ifdef:ed"

Some Questions:

Why do we need gnome_program call? only reason right now is buggy session manager by gtk/glib and so on. Or I am missing something? :-)

2. Is the gedit in trunk marked as unstable? Could we start implement icon support from gio glib branch? To get rid of gnome_icon_lookup calls totally and kill those ifdef:s to :-)
Comment 5 Mikael Hermansson 2007-12-02 18:21:23 UTC
Created attachment 100056 [details] [review]
Kills gnome_help call

Kills gnome_help and replaces it with gdk_spawn* and lookup correct help files.

Code based on evince help code.
Comment 6 Mikael Hermansson 2007-12-02 18:34:51 UTC
Created attachment 100057 [details] [review]
Get rid of gnome_uitl calls

get rid of gnome_util calls in favor of g_*
Comment 7 Mikael Hermansson 2007-12-09 17:04:05 UTC
Created attachment 100651 [details] [review]
New patch

* gnome_help used as default if libgnome is linked
* gnome-session has no ifdef instead ifdef it in Makefile.am
* gnome_url* ifdef added in /plugin/filebrowser/gedit_file_browser_widget.c
* and other small fixes
Comment 8 Jani Monoses 2007-12-09 17:13:39 UTC
Mikael,

I've filed a few help_display removing patches in the 2.20 cycle and one in 2.22
Here's the one for gucharmap, it handles i18n as it is lifted from libgnome
It was done similarly in gcalctool network-manager and a few others IIRC

http://bugzilla.gnome.org/attachment.cgi?id=94089&action=view

This part then can be made unconditional as it is behaving exactly like libgnome

For handling gnome_url see this patch in the first attachment that was applied to nm (gucharmap and nm lost libgnome deps in 2.22)

http://bugzilla.gnome.org/show_bug.cgi?id=460671
Comment 9 Paolo Borelli 2007-12-09 17:36:58 UTC
Jani, I was the one asking Mikael to put back gnome_help conditionally... I am all for killing it once we get rid of libgnome for good, but until it is still there it would be silly to not use the library function and enjoy the potential bugfixes done in the lib.

For gnome_url we can also easily use gnome_vfs_url_show for now, the problem is the error handling since gnome_url_show proviedes accurate error messages for different kind of errors. It will be all properly solved when switching to gio...
Comment 10 Jani Monoses 2007-12-09 17:52:51 UTC
Paolo,

sure, your call :) I doubt though there are going to be bugfixes in these straightforward functions to warrant keeping libgnome for them onnly. My interest
for dropping libgnome deps is improving startup speed and memory footprint,
so whether 1 API or 5 are used does not matter much as long as libgnome & co are still linked at runtime.
The startup speed is noticable for all apps, the footprint gains are admittedly more useful for long living apps as g-v-m and the other manager apps.
Comment 11 Mikael Hermansson 2007-12-09 19:36:47 UTC
Created attachment 100658 [details]
New patch fixing gnome_vfs_show_uri

New patch 

* fixing gnome_vfs_show_uri in filebrowser
* Add default icons in filebrowser when --wihout-gnome compile
Comment 12 Mikael Hermansson 2007-12-10 01:14:09 UTC
Created attachment 100664 [details] [review]
add g_option_context_parse if libgnome deprecated

* add g_option_context_parse if libgnome disabled
Comment 13 Mikael Hermansson 2007-12-16 17:21:00 UTC
Created attachment 101062 [details] [review]
Small fix to timeplugin

* fix timeplugin to use g_key_file instead of gnome_config for the state.
Comment 14 Steve Frécinaux 2007-12-26 12:28:03 UTC
> 	if (file_name == NULL)
>-		file_name = "gedit.xml";
>+		file_name="gedit.xml";

Great patches indeed , but you should follow more closely gedit's coding conventions...
Comment 15 Mikael Hermansson 2007-12-26 14:32:18 UTC
Created attachment 101621 [details] [review]
This patch is in sync with SVN version of gedit:

This patch is in sync with SVN version of gedit:

Timeplugin/spellcheck patch removed because theyr already patched/changed in SVN to not use libgnome et all :-)

This patch ifdefs:

* gnome_program   
* session managment  (gedit_session.c/h not compiles ifndef ENABLE_LIBGNOME)
* replace gnome_help with gdk_spawn based on evince code ifndef ENABLE_LIBGNOME.
* filebrowser plugin gnome_icon_lookup replaced with default icons for files/directorys maybe we should kill gnome_lookup and user default file/diricon even if libgnome is compiled and replace it with GIO later on. 

(I have a test filemodel written in vala using GIO stuff maybe we could use that model? (if I rewrite the model in C ofcourse?)
Comment 16 Martin Meyer 2007-12-26 17:05:18 UTC
From the comments it looks like you're still looking for session management code to replace the libgnome stuff. Havoc has told me before[1] that Xlib has facilities for this, and there's some code in Metacity you should be able to copy. Hope that helps.

I would suggest that for anything in your patch that does not regress without using libgnome, you should remove the libgnome version entirely. There's no point in letting it be done the libgnome way if we have a more modern/current/maintained way available.

[1] http://mail.gnome.org/archives/desktop-devel-list/2007-July/msg00060.html
Comment 17 Mikael Hermansson 2007-12-26 20:16:51 UTC
The problem with the session manement has been discussed for years now.

There are code in libegg module for session managment that could replace the old session stuff. But then we still depends on a new library and Imho this is stupid. 

If users want session stuff then there should be something in Gtk+ for that. We dont need copy&paste or "yet another library" :-)

But the patch above was for those who dont care about the sessionmanagment and want to kill libgnome/bonobo dependies using --without-libgnome. 

Still we cannot kill libgnome totally until upstream has replacement for gnome_help/gnome_icon_lookup/session stuff with gtk+/glib replacements. 

1. The gnome_icon_lookup could infact be killed and replaced with patch that will set the icon to default theme's file/directory icon for ALL files/dirs in tab/filebrowser plugin.

That way we kill all #ifdefs in filebrowser. I dont think we implemented any new bugs by doing this because its a very small change (as you can see in the patch) and it work here without problems.

As soon as glib is stable we could replace default icons with gio API for those files that has a non default file icons :-)

2. About the gnome_help #ifdefs pbor did'nt want to kill gnome_help until we are sure the new code works. So people should test the new code using configure --without-libgnome and test the replacement gedit_help code. To make sure it works and hopefully we could kill gnome_help ifdefs too :-)

(The help replacement code is taken from evince codebase)


 
Comment 18 Jani Monoses 2007-12-27 21:51:05 UTC
the gnome_help code as replaced in g-p-m, gcalctool, g-s-t, n-m and many others definitely works, it is lifted form libgnome. It is copy paste, granted but a small price to pay iff it means libgnome dependency can be avoided.
Comment 19 Mikael Hermansson 2008-02-04 21:50:05 UTC
Created attachment 104435 [details] [review]
New patch that works with SVN trunk

New patch that works with SVN trunk after remove of libgnomeprintings its time to kill libgnome/ui :-) (must specify --without-libgnome as before)
Comment 20 Paolo Borelli 2008-08-13 17:18:52 UTC
We gather here today to mourn the demise of the libgnome/ui dependency.