GNOME Bugzilla – Bug 671090
Remove historical stuff from the jhbuild wrapper script
Last modified: 2012-04-24 19:21:52 UTC
See the patch for details. Not strictly necessary, but it should improve common workflow. Also, it fixes some subtle bugs caused by duplicate XDG_DATA_DIRS.
Created attachment 208726 [details] [review] jhbuild wrapper: drop a lot of historical stuff - dconf exists on the bus since 2.32, no need to run it from the launcher script. - notification-daemon is part of the fallback session (which kind of doesn't exist anymore, now that gnome-shell runs on software rendering), so it's unlikely to be around. - Since we moved to the main jhbuild moduleset, it is expected people will use jhbuild shell or jhbuild run to run the shell, so a lot of environment variables are useless or actively harmful (like XDG_*_DIRS containing the jhbuild prefix twice) - Similarly, since gnome-shell is now in PATH, trying to "restore GNOME" after a crash is just waiting for the same crash (especially for stuff like syntax errors). Also, it detaches the process from the terminal, which is quite bad.
Review of attachment 208726 [details] [review]: Could we take the perf stuff and put that in a separate script, similar to how we split out gnome-shell-extension-tool?
Created attachment 210575 [details] [review] jhbuild wrapper: drop a lot of historical stuff - dconf exists on the bus since 2.32, no need to run it from the launcher script. - notification-daemon is part of the fallback session (which kind of doesn't exist anymore, now that gnome-shell runs on software rendering), so it's unlikely to be around. - Since we moved to the main jhbuild moduleset, it is expected people will use jhbuild shell or jhbuild run to run the shell, so a lot of environment variables are useless or actively harmful (like XDG_*_DIRS containing the jhbuild prefix twice) - Similarly, since gnome-shell is now in PATH, trying to "restore GNOME" after a crash is just waiting for the same crash (especially for stuff like syntax errors). Also, it detaches the process from the terminal, which is quite bad. - Finally, all performance stuff has been moved to a separate script and ported to newer pygobject + gdbus.
Review of attachment 210575 [details] [review]: ::: src/gnome-shell-jhbuild.in @@ +177,3 @@ options.debug_command = "gdb --args" +sys.exit(0 if run_shell() else 1) Ew. Keep it the way it was. ::: src/gnome-shell-perf-tool.in @@ +78,3 @@ + self_dir = os.path.dirname(os.path.abspath(sys.argv[0])) + args.append(os.path.join(self_dir, 'gnome-shell')) + # pass on any additional argument arguments. @@ +94,3 @@ + +def upload_performance_report(report_text): + # Local imports to avoid impacting gnome-shell startup time What local imports?
Created attachment 210581 [details] [review] jhbuild wrapper: drop a lot of historical stuff - dconf exists on the bus since 2.32, no need to run it from the launcher script. - notification-daemon is part of the fallback session (which kind of doesn't exist anymore, now that gnome-shell runs on software rendering), so it's unlikely to be around. - Since we moved to the main jhbuild moduleset, it is expected people will use jhbuild shell or jhbuild run to run the shell, so a lot of environment variables are useless or actively harmful (like XDG_*_DIRS containing the jhbuild prefix twice) - Similarly, since gnome-shell is now in PATH, trying to "restore GNOME" after a crash is just waiting for the same crash (especially for stuff like syntax errors). Also, it detaches the process from the terminal, which is quite bad. - Finally, all performance stuff has been moved to a separate script and ported to newer pygobject + gdbus.
Review of attachment 210581 [details] [review]: OK -- while I might be fine with stuffing this into the Shell as one patch, I'd really prefer if it was split up into multiple commits. Each item in the commit message should be one patch, along with a generic "cleanup" patch that removes unused imports and other stuff like that. With a bunch of patches rather than just one, we can probably land some of this in 3.4.1 much easier.
Ok, I splitted it into multiple commits. I guess it will be easier to review as a branch, than as bugzilla patches, so you can find it at https://github.com/gcampax/gnome-shell/tree/jhbuild-wrapper
(In reply to comment #7) > Ok, I splitted it into multiple commits. > I guess it will be easier to review as a branch, than as bugzilla patches, so > you can find it at https://github.com/gcampax/gnome-shell/tree/jhbuild-wrapper It all looks fine to me. ACN as far as I'm concerned.
Thanks, committed to master.