GNOME Bugzilla – Bug 704215
Make System Monitor work in Wayland
Last modified: 2013-07-16 20:46:01 UTC
Currently System Monitor links against libwnck, which in turn as X as a dependency. GSM uses libwnck for 1. Obtain the application icon in some corner cases. 2. Obtain the X memory usage of the application. I admit I have not investigated thoroughly, but I think that 1. is kind of obsolete function in the .desktop file era, so I propose to just remove that code. As for 2., it seems useful when running under X, so it could be made configurable at build time. Patched attached that implement this.
Created attachment 249140 [details] [review] Do not use libwcnk for obtaining icons
Created attachment 249141 [details] [review] Make libwnck opt-in Use libwnck only if the configure flag '--enable-wnck' is specified.
Created attachment 249142 [details] [review] Remove useless include
Why can't we wrap the wnck icon stuff with a conditional if, just like we do for the proctable wnck-dependant part? Even if it's only for some corner cases, it might be useful in some cases. So the second and third patch are OK, I'm not sure about the first one, I'd rather have that code also tied to --enable-wnck b (in lack of deeper investigation - I am also not aware where it would be the only option to get the icon)
Review of attachment 249142 [details] [review]: Thanks for the cleanup.
Created attachment 249179 [details] [review] Make libwnck opt-in Use libwnck only if the configure flag '--enable-wnck' is specified.
New patch with icon code ifdef'd
Review of attachment 249179 [details] [review]: Thanks for the quick fix. Looks better, basically would be OK, but it unfortunately breaks the default columnset with wnck off, as the column numbers after the Xserver memory column get shifted. I'm working on fixing this and will commit afterwards, but if you have any quick suggestion on how this can be elegantly solved, I'm open for suggestions.
Created attachment 249249 [details] [review] Make libwnck opt-in Use libwnck only if the configure flag '--enable-wnck' is specified. --- Ah, I didn't notice that at first... I think then that the best option is to just avoid showing the Xserver memory column, instead of removing it from the model (doing that in this new patch). This should preserve all the columnset data.
(In reply to comment #10) > Created an attachment (id=249249) [details] [review] > Make libwnck opt-in > > Use libwnck only if the configure flag '--enable-wnck' is specified. > > --- > > Ah, I didn't notice that at first... I think then that the best option is > to just avoid showing the Xserver memory column, instead of removing it > from the model (doing that in this new patch). This should preserve all the > columnset data. Sorry, too rushed... this patch is wrong... Have you already done work in fixing the issue? Otherwise I'll try to get it correct
Yes, I have tried "ignoring" the column with ifndef but I have got into some segfaults, which I couldn't sort out yet, and I will not be able today either, so the patch is welcome, after reviewing it I'll commit it, I'm sure we can sort that out until 3.9.5 release. Thanks for your help anyway, it's welcome.
Created attachment 249266 [details] [review] Make libwnck opt-in Use libwnck only if the configure flag '--enable-wnck' is specified. This should be the correct one, I hope :) The problem was some weird check in procman_get_tree_state which used the column number instead of the column id, messing up the ordering.
Review of attachment 249266 [details] [review]: Thanks, this looks fine, tested the migration and made some basic tests, as everything went fine, I've committed it.
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.
You didn't really mean to push a random 'src/nextmon' file, right? :) Maybe you already know, but there's a useful tool by Owen Taylor for dealing with bugzilla patches, git-bz (http://fishsoup.net/software/git-bz/)
(In reply to comment #16) > You didn't really mean to push a random 'src/nextmon' file, right? :) Thanks, I completely forgot that, and that's a file that wasn't pushed to the other branch, so, it got away :) > Maybe you already know, but there's a useful tool by Owen Taylor for dealing > with bugzilla patches, git-bz (http://fishsoup.net/software/git-bz/) No, I didn't know about that, but I'll check it out. My current workflow was to wget the patch file, and git apply, fix as necessary, git add and commit. Seems like git-bz will help me a bit there. Thanks for the tip.