search

linuxmint / cinnamon-spices-applets

Applets for the Cinnamon desktop
http://cinnamon-spices.linuxmint.com
606 stars 509 forks source link

[hwmonitor@sylfurd]: Can not add to panel #3023

Open jkirk opened 4 years ago

jkirk commented 4 years ago

Facts

Notify author of applet @sylfurd, @hultan, @claudiux (not listed in CODEOWENERS)

Issue

The applet can not be added to the panel.

There was a problem loading the selected item, and it has been disabled.

The relevant part of~/.xsession-errors looks like this:

Cjs-Message: 20:07:01.690: JS LOG: [LookingGlass/error]
[hwmonitor@sylfurd]: Requiring GUdev, version none: Typelib file for namespace 'GUdev' (any version) not found
[hwmonitor@sylfurd]: Error importing applet.js from hwmonitor@sylfurd
Cjs-Message: 20:07:01.690: JS LOG: [LookingGlass/trace]
<----------------
anonymous@/home/jkirk/.local/share/cinnamon/applets/hwmonitor@sylfurd/3.8/applet.js:32:7
createExports@/usr/share/cinnamon/js/misc/fileUtils.js:210:31
requireModule/</<@/usr/share/cinnamon/js/misc/fileUtils.js:288:25
---------------->

Thanks for looking into this.

Hultan commented 4 years ago

Hey @jkirk

I am pretty sure I know what this problem is, hwmonitor now has a new dependency to a file called, on libgudev (at least on Manjaro/Arch). This was to fix a problem with the disk graphs under some circumstances. We were not sure if this was installed already with Cinnamon, but it might not be.

Can you try and install libgudev, hopefully you will find it in your distribution. I believe it is this one, but I don't use Debian so I don't know for sure : https://packages.debian.org/source/stretch/libgudev

@jacobwills do you want to fix this? Give a better error message, and a recommendation to install the file like we do for the other dependency libgtop? Alternatively we could switch to the old way of doing the disk graphs if we get this error. I am pretty sure I can look at it this weekend, if you are busy.

jacobwills commented 4 years ago

@Hultan Sure I can take a look at getting the fix you detailed implemented by mirroring the libgtop dependency check and notice to users if it's not installed for libgudev as well. I was also looking recently at ways we can help the user fetch and install the dependent packages (look at SpicesUpdate to see how they are doing it there) but that is going to have to wait for now as I have limited time to work on this at the moment.

I'll have to see about finding a way to fallback to the previous method if the new one fails somehow if you think that is necessary, although I'm not entirely in support of this approach. IMHO, the user should just install the packages they need to run the software we've written instead of us trying to maintaining multiple ways of handling different scenarios like this. It's going to end up taking a lot more code work in the end if we continue that trend. I think a dependency notice should suffice for now. Just my two cents. Let me know what you think.

Actually this might be better: we could find out which version of Cinnamon desktop introduced libgudev as a dependency (at least that seems to be the case for me; I'm currently running/developing on Cinnamon 4.4.8) and then we create a new applet version which uses the new method I committed recently while the older versions (3.8.8 as @jkirk stated he is running) can continue to use the older method if libgudev is not part of the base Cinnamon installation at that point. That might be an even better approach and is my recommendation but it will require a little more research to find out which Cinnamon version we need to branch a new applet version at exactly.

@Hultan Let me know what you think we should do before I get started on anything. Thanks!

@jkirk Can you please confirm that installation of the libgudev package provided by your distribution fixed the issue you were having with this applet before we start working on or commit a fix for this issue? Also can you try grabbing the version at this commit 520f9f7 to see if that works for you instead? That may help us to decide if a new applet version might be a better idea for you Debian folks while allowing those with newer versions of Cinnamon to continue to use the new disk stats implementation. Thanks!

FYI: I am pretty busy at the moment. The sale of our old house closes on the 22nd and we are busy packing and moving everything out and will be driving 12-14 hours with our dogs to move in to our new home the following week... Anyway, maybe I can get something done this weekend, but no promises.

Hultan commented 4 years ago

I just want to clarify that I am not the author of this applet, I just finally entered my name in the author tag in info.json, after it had been empty for the year, or so, that I had pretty much worked on this applet alone. I figured it would make it easier for me, if I get notifications whenever an issue is added to the applet. So I have no authority here :-)

Basically, do whatever you feel is right whenever you have time. I just figured it would be easier for you, since you made the switch to libgudev...If I find the time before you do, I might take stab at it, at least the "easier solution" to just pop up a notification...

jkirk commented 4 years ago

Hi all,

thanks for your support!

Can you try and install libgudev, hopefully you will find it in your distribution. I believe it is this one, but I don't use Debian so I don't know for sure : https://packages.debian.org/source/stretch/libgudev

Installing libgudev-1.0-0 (GObject-based wrapper library for libudev) is not enough, but I can confirm, that installing gir1.2-gudev-1.0 (GObject-introspection data of libgudev-1.0) fixed the problem. I just created a PR to update the README.rst to reflect that dependency.

@Hultan Sure I can take a look at getting the fix you detailed implemented by mirroring the libgtop dependency check and notice to users if it's not installed for libgudev as well. I was also looking recently at ways we can help the user fetch and install the dependent packages (look at SpicesUpdate to see how they are doing it there) but that is going to have to wait for now as I have limited time to work on this at the moment.

Can you point us at the libgtop dependency check. I / we might find some time to mirror that check.

I'll have to see about finding a way to fallback to the previous method if the new one fails somehow if you think that is necessary, although I'm not entirely in support of this approach. IMHO, the user should just install the packages they need to run the software we've written instead of us trying to maintaining multiple ways of handling different scenarios like this. It's going to end up taking a lot more code work in the end if we continue that trend. I think a dependency notice should suffice for now. Just my two cents. Let me know what you think.

I agree. Keep it simple. Just tell the user what to do if a dependency is not met (and if possible do not crash hard).

Actually this might be better: we could find out which version of Cinnamon desktop introduced libgudev as a dependency (at least that seems to be the case for me; I'm currently running/developing on Cinnamon 4.4.8) and then we create a new applet version which uses the new method I committed recently while the older versions (3.8.8 as @jkirk stated he is running) can continue to use the older method if libgudev is not part of the base Cinnamon installation at that point. That might be an even better approach and is my recommendation but it will require a little more research to find out which Cinnamon version we need to branch a new applet version at exactly. @Hultan Let me know what you think we should do before I get started on anything. Thanks!

Hmm. Cinnamon (3.8.8 and 4.4.8) does not seem to depend on libgudev / gir1.2-gudev-1.0, neither in Debian/buster, Debian/bullseye nor Ubuntu 20.04LTS. Where did you take the dependency information from?

@jkirk Can you please confirm that installation of the libgudev package provided by your distribution fixed the issue you were having with this applet before we start working on or commit a fix for this issue? Also can you try grabbing the version at this commit 520f9f7 to see if that works for you instead? That may help us to decide if a new applet version might be a better idea for you Debian folks while allowing those with newer versions of Cinnamon to continue to use the new disk stats implementation. Thanks!

Yes, installing gir1.2-gudev-1.0 fixes the problem for me. I haven't tried rev 520f9f7, but if you think it (still) helps to do so, let me know.

FYI: I am pretty busy at the moment. The sale of our old house closes on the 22nd and we are busy packing and moving everything out and will be driving 12-14 hours with our dogs to move in to our new home the following week... Anyway, maybe I can get something done this weekend, but no promises.

No worries. I am fine for now. Good luck in your new home and thank you for your support!

jacobwills commented 4 years ago

@jkirk @Hultan

Can you point us at the libgtop dependency check. I / we might find some time to mirror that check.

You can find that here: https://github.com/linuxmint/cinnamon-spices-applets/blob/c5e1fde247b4c2cd06a182d4f352f4c7b11521de/hwmonitor%40sylfurd/files/hwmonitor%40sylfurd/3.8/providers.js#L12

It should be easy enough to replicate that try...catch block into applet.js (since that contains the first import encountered at runtime of libgudev. See here: https://github.com/linuxmint/cinnamon-spices-applets/blob/c5e1fde247b4c2cd06a182d4f352f4c7b11521de/hwmonitor%40sylfurd/files/hwmonitor%40sylfurd/3.8/applet.js#L32)

NOTE: You may also want to put the info added by your last commit to the README.md into these messages displayed to the user when the check(s) fail. Looks like that's appropriate place for it to be updated as well. You could also revise the names of the packages to match exactly the name used by each distribution's repositories (or not, they are only slightly different. Doing so may make it easier for automated install later on if we do implement that feature I guess).

Hmm. Cinnamon (3.8.8 and 4.4.8) does not seem to depend on libgudev / gir1.2-gudev-1.0, neither in Debian/buster, Debian/bullseye nor Ubuntu 20.04LTS. Where did you take the dependency information from?

It appeared to be implied by the following dependencies/requirements trees:

I'd need to go back through the change histories to narrow down which version of Cinnamon this was introduced with but this appears to be it to me.

Tell me if I am misinterpreting this and/or if this is significantly different for other distributions and/or versions of Cinnamon. Maybe I'm wrong, but I thought that would pretty much ensure that if you're running a Cinnamon session you would have libgudev installed.

Installing libgudev-1.0-0 (GObject-based wrapper library for libudev) is not enough, but I can confirm, that installing gir1.2-gudev-1.0 (GObject-introspection data of libgudev-1.0) fixed the problem.

Curiously, in your case (on Debian), that apparently wasn't enough so and you had to also install gir1.2-gudev-1.0 explicitly... Did you try restarting Cinnamon after installing that first package? I'm not sure that would make any difference at all but sometimes...

Hmm... I'm not too sure about why Debian is packaging things this way. There isn't an equivalent ("gir" specific) package on Arch that I can see. The more I look into this, I wonder if what really provides access to those libraries for us here is the gobject-introspection-runtime package across the board in addition to the libraries we are talking about, as that is required by Cjs/Gjs (at least as packaged for those distributions I researched so far, listed above). The runtime should be how the GObject language bindings for Gjs get access to and use the introspection data/types of libgudev and libgtop. GObject Introspection

I'm not 100% sure I understand the relationships/distinctions between these enough to make some determination as to why these are packaged differently across different distributions OR more importantly, how we can/should go about checking for the correct packages properly within the our applet here... I'll need to read a little more about it for it to sink in...

That's all I have time for at the moment though. Thanks for pitching in! Cheers!