GNOME Bugzilla – Bug 641724
gnome-shell's launcher script shouldn't be in Python
Last modified: 2011-03-14 15:57:12 UTC
The src/gnome-shell executable is currently a Python script. I do love Python very much, but having to launch a VM to start an app which using another VM is pretty silly.
The majority of the script is just to deal with running GNOME Shell in a GNOME 2.x environment (or running it from the source tree). But yeah, I'd always been assuming that it would eventually turn into something close to: #!/bin/sh exec mutter --mutter-plugins=libgnome-shell "$@" The --xephyr stuff is going to go away. The --perf and --create-extension stuff could go into separate scripts. It wouldn't be hard to do the rest as a shell script.
(In reply to comment #1) > The majority of the script is just to deal with running GNOME Shell in a GNOME > 2.x environment (or running it from the source tree). But yeah, I'd always been > assuming that it would eventually turn into something close to: > > #!/bin/sh > > exec mutter --mutter-plugins=libgnome-shell "$@" > > The --xephyr stuff is going to go away. The --perf and --create-extension stuff > could go into separate scripts. It wouldn't be hard to do the rest as a shell > script. Even better, the --create-extension can be removed, now that gnome-shell-extensions ships the same example.
the gnome-shell python script shows up very unfavourably in bootchart (for example, here: http://www.harald-hoyer.de/personal/blog/2011-02-03-fedora-rawhide-bootcharts -- but that's not even a slow machine...), especially on slower hw and on rotating media. Pulling in Python is a recipe for making the boot slow. Also, invoking glxinfo from the script makes it even worse. It would be great if the python script could go away. On fedora we have been trying hard to speed things up by removing shells from the boot process. Due to that we'd much appreciate if the python script would not be replaced by a shell script, but would simply go away entirely, and g-s/mutter could just be invoked like any other C program.
(In reply to comment #3) > On fedora we have been trying hard to speed things up by removing shells from > the boot process. Due to that we'd much appreciate if the python script would > not be replaced by a shell script, but would simply go away entirely, and > g-s/mutter could just be invoked like any other C program. Hm... I was going to say that we can't easily do that (unless we had a C program that was just the equivalent of the above shell script), but I guess it would be pretty trivial to just build a "libmutter" with -Dmain=mutter_main, and then have gnome-shell be a binary that links to libmutter, does its own initialization, adds "--mutter-plugins=libgnome-shell" to argv and then calls mutter_main(). And then we could make it progressively saner from there. (Having the "plugin" be in process rather than dlopened, splitting mutter's main into separate mutter_init() and mutter_run() sections so we could move the initialization stuff from gnome-shell-plugin.c into main instead, etc.)
Another simple solution would be to simply fork off and exec mutter from g-s with the right command line. If there's value in having g-s and mutter be two processes that would be a pretty good solution. I am mostly pushing for eradication of python and shells scripts from our default boot path. Whether mutter/g-s are two or one processes doesn't really matter too much...
It would be just exec, not fork+exec. The problem isn't multiple processes, it's that the actual thing-which-is-GNOME-Shell is not a standalone binary, it's a plugin that gets loaded by mutter. So (as things are organized now) if there's a /usr/bin/gnome-shell, it would be something that execs mutter with the correct arguments to cause it to load libgnome-shell.so.
Ah, I see, so there is no such thing as a gnome-shell binary? Maybe a solution would be to add some code to mutter to simply check whether argv[0] includes "gnome-shell" and in this case just hardcode that libgnome-shell.so is loaded? Or, just give up on the gnome-shell executable name and just directly invoke mutter with the right parameters everywhere, like in the .desktop file.
See also bug 642084.
*** This bug has been marked as a duplicate of bug 642084 ***
reopening to consider the possibility of actually getting rid of the need for a wrapper
Created attachment 182373 [details] [review] initial libmutter work
oops, I apparently never committed the working state of the gnome-shell side of that patch, and only have a later, broken, state now. The idea here is: make the absolute minimal change to turn mutter into a library, and then make a tiny tiny gnome-shell binary that just links to that library (but handles things like setting NO_AT_BRIDGE and adding --mutter-plugins=libgnome-shell.la to argv before calling meta_main, and which can be forcibly linked-with-rpath to the right libmozjs, per bug 643799, and which will save us having to repeatedly explain why "why does gnome-shell only work with mutter?" is not even a meaningful question) Step two was going to be to get rid of the dynamic loading of the plugin, and just link what is currently libgnome-shell.la into the gnome-shell binary, but my first attempt at that didn't work, because the introspection was getting messed up somehow (the running binary couldn't find St) and then I didn't get back to it. Thoughts?
Created attachment 182382 [details] [review] the mutter side of step 2
Created attachment 182383 [details] [review] the non-working gnome-shell side of the step 2 mutter patch. this predates Colin's wrapper script changes and won't apply cleanly to HEAD
Review of attachment 182373 [details] [review]: Hah, a very simple patch. How hard would it be though to change main.c to call mutter_main()? The way things are right now, everything gets built twice.
(In reply to comment #15) > Hah, a very simple patch. How hard would it be though to change main.c to call > mutter_main()? The way things are right now, everything gets built twice. Well, I wasn't sure if we would want to keep the mutter binary around at all in this case.
(In reply to comment #16) > (In reply to comment #15) > > Hah, a very simple patch. How hard would it be though to change main.c to call > > mutter_main()? The way things are right now, everything gets built twice. > > Well, I wasn't sure if we would want to keep the mutter binary around at all in > this case. We've been using it for testing, so it seems to have some value, and it can't be that hard to keep around is it?
No, I just meant, I didn't know if we were going to decide "we should keep the mutter binary and make it link against libmutter-wm" or "we should ditch the mutter binary", and so I didn't make any attempt to implement either of those for now.
Having raw mutter available has been useful to try and sort out different classes of bugs, so I think it's worth a small amount of effort to maintain it.
(In reply to comment #19) > Having raw mutter available has been useful to try and sort out different > classes of bugs, so I think it's worth a small amount of effort to maintain it. Though, we could also have a gnome-shell command line options to disable various things (--disable-javascript --disable-animations).
Review of attachment 182373 [details] [review]: I think this approach is worth pursuing - going to make the system look more "real" and being able to use RPATH will likely help with spidermonkey for gnome-shell. (Though doesn't help at all if people people put libmozjs.so in a directory that moves with ffox security updates.) I'd like to keep the "mutter" binary around, so it's worth the extra few lines of Makefile.am changes and the mutter-main.c file to make everything not build twice.
Review of attachment 182382 [details] [review]: Seems OK as an intermediate step to some less pluginified future. Will need work for changes in the preceding patch requested to make mutter use libmutter-wm ::: src/include/meta-plugin.h @@ +223,2 @@ void +meta_plugin_register (MetaPlugin *plugin); I think i'd call this (and the MPM method) install, not register
Review of attachment 182383 [details] [review]: Not sure if there are other problems, but you've clearly killed the --introspect functionality. You need to do something like do a manual preparse of the argv and look for --introspect and then call g_irepository_dump() yourself. Or alternatively, make the init()/register()/run() thing work for --introspect mode
Created attachment 182557 [details] [review] Use libmutter-wm, and build a real gnome-shell binary Build gnome-shell as a binary linked against libmutter-wm, instead of a module to be loaded by libmutter-wm. Move the majority of initialization-type stuff from gnome_shell_plugin_start() into main(). We still build libgnome-shell as a shared library, so that the linker doesn't discard all the methods that are never called from C. === see bug 643959 for the corresponding mutter patch
Created attachment 182558 [details] [review] src: update for mutter include reorganization
(In reply to comment #23) > Review of attachment 182383 [details] [review]: > > Not sure if there are other problems, but you've clearly killed the > --introspect functionality. Yes, but it doesn't matter, because we don't introspect via --program any more.
Review of attachment 182557 [details] [review]: > We still build libgnome-shell as a shared library, so that the linker doesn't discard all the methods that are never called from C. The -export-dynamic flag should take care of that. It's used in Mutter currently and if it wasn't working, then we'd be unable to add a new method to Mutter that is used only from gnome-shell and then use it from gnome-shell, and we do that all the time. ::: src/Makefile.am @@ +18,3 @@ CLEANFILES += $(gir_DATA) $(typelib_DATA) +bin_SCRIPTS += gnome-shell gnome-shell-extension-tool Is using bin_SCRIPTS for an executable going to break the relinking thing that libtool does on installation? Do we risk installing a libtool wrapper script or a binary with a rpath to the build directory? Maybe we should always have gnome-shell-real in bin_PROGRAMS then have install-exec-hook move gnome-shell-real to gnome-shell? @@ +58,3 @@ gnome_shell_cflags = \ + $(GNOME_SHELL_CFLAGS) \ + $(BLUETOOTH_CFLAGS) \ I don't see anything in configure.ac that would cause BLUETOOTH_CFLAGS to be defined (in general, the existence of <SOME_DEP>_CFLAGS is a bug from improper use of pkg-config, but the bluetooth stuff is pretty messy, so maybe it's necessary. But still don't see where it is set.) ::: src/gnome-shell-jhbuild.in @@ +167,3 @@ # run". See bug #642084 env.update({'GNOME_SHELL_JS' : js_dir, + 'PATH' : '@bindir@:' + os.environ.get('PATH', ''), Hmm, it's pretty confusing to have @bindir@ and bin_dir be different things. Maybe it's OK though, since @bindir@ has a well established meaning. Possibly consider renaming bin_dir to executable_dir or something? (just a thought) @@ +499,3 @@ help="Run under a debugger") parser.add_option("", "--debug-command", metavar="COMMAND", + help="Command to use for debugging (defaults to 'libtool --mode=execute gdb --args')") I think the libtool --mode=execute should be added automatically to whatever debug command is passed when running uninstalled. If I want to valgrind I should be able to 'gnome-shell --debug-command=valgrind' not 'gnome-shell --debug-comand="libtool --mode=execute valgrind"' ::: src/main.c @@ +2,3 @@ + +/* + * Copyright (c) 2011 Red Hat, Inc. no '(c)' Not sure what is best here - this is accurate in that there is some small amount of stuff here which is copyright 2011 Red Hat, but there's a fair bit of stuff from other copyright holders and most stuff is from other years. (Less concerned about the other years... I think I've even seen recommendations that you can just put the last year.) Don't have a better recommendation other than tracking down copyright holders fully, which it probably doesn't make sense to do for one file in isolation. ::: tests/run-test.sh.in @@ +31,3 @@ srcdir=`cd $srcdir && pwd` +GI_TYPELIB_PATH="@MUTTER_TYPELIB_DIR@:$builddir/../src" You need to adjust tests/Makefile.am for this
Review of attachment 182558 [details] [review]: Looks fine. There's still a gap between the metacity headers and what would be OK for a real first class library, but its fine to group things as if it were such a thing.
(In reply to comment #27) > > We still build libgnome-shell as a shared library, so that the linker doesn't discard all the methods that are never called from C. > > The -export-dynamic flag should take care of that. -export-dynamic causes all of gnome-shell's own unused symbols to be kept (eg, shell_generic_container_*), but doesn't cause libst, libgvc, etc, symbols to be kept. -whole-archive might work for that, but it's incompatible with automake. Another possibility would be to get rid of all of the intermediate libraries, and just link everything directly into gnome-shell directly... > Is using bin_SCRIPTS for an executable going to break the relinking thing that > libtool does on installation? Meh. Probably. > I don't see anything in configure.ac that would cause BLUETOOTH_CFLAGS to be > defined Yes, I saw BLUETOOTH_LIBS and assumed they had just forgotten BLUETOOTH_CFLAGS, and didn't notice because it was just "-I$(prefix)" which we already had. But yes, it doesn't exist. (Nor does $(LIBECAL_CFLAGS), which appears elsewhere...) > consider renaming bin_dir to executable_dir or something? (just a thought) I renamed it to self_dir, since it's the directory where the script is. > + * Copyright (c) 2011 Red Hat, Inc. > > no '(c)' oops, I just copied it from mutter/src/core/mutter.c, which I copied from mutter/src/core/main.c... Maybe I'll just remove it, since we don't have copyright notices on most of the other files...
(In reply to comment #29) > (In reply to comment #27) > > > We still build libgnome-shell as a shared library, so that the linker doesn't discard all the methods that are never called from C. > > > > The -export-dynamic flag should take care of that. > > -export-dynamic causes all of gnome-shell's own unused symbols to be kept (eg, > shell_generic_container_*), but doesn't cause libst, libgvc, etc, symbols to be > kept. -whole-archive might work for that, but it's incompatible with automake. > > Another possibility would be to get rid of all of the intermediate libraries, > and just link everything directly into gnome-shell directly... Ugh. Don't think it's worth a lot of fiddling - I can't think of any real harm from the private library approach other than maybe a tiny bit of slowdown from -fPIC code and being unable to set breakpoints in gdb before main() is hit.
Created attachment 182626 [details] [review] Use libmutter-wm, and build a real gnome-shell binary Build gnome-shell as a binary linked against libmutter-wm, instead of a module to be loaded by libmutter-wm. Move the majority of initialization-type stuff from gnome_shell_plugin_start() into main(). We still build libgnome-shell as a shared library, so that the linker doesn't discard all the methods that are never called from C.
Created attachment 182627 [details] [review] src: update for mutter include reorganization
Review of attachment 182626 [details] [review]: Looks good to me
Review of attachment 182627 [details] [review]: Looks good
Attachment 182626 [details] pushed as ae96b0c - Use libmutter-wm, and build a real gnome-shell binary Attachment 182627 [details] pushed as e187961 - src: update for mutter include reorganization
Created attachment 183350 [details] [review] Simplify and bashify gnome-shell-extension-tool Python is not really needed here as Bash can easily do all the work
* Don't attach patches to closed bugs, open a new bug. * What's the advantage? GNOME Shell is going to keep on shipping Python utility scripts for performance testing, so having a Python script for extension creation seems like not a big deal. * I could certainly imagine this getting more complex over time.