Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More debug statements #305

Merged
merged 12 commits into from
Jul 27, 2024
Merged

More debug statements #305

merged 12 commits into from
Jul 27, 2024

Conversation

nokyan
Copy link
Owner

@nokyan nokyan commented Jul 25, 2024

Resolves #298

@nokyan nokyan mentioned this pull request Jul 25, 2024
1 task
@jojo2357
Copy link
Contributor

Cant build due to errors around lazy_cell

@nokyan
Copy link
Owner Author

nokyan commented Jul 26, 2024

Cant build due to errors around lazy_cell

Updating your Rust toolchain should fix this, Rust 1.80 includes LazyLock in std, Resources uses this now instead of lazy_cell.

@jojo2357
Copy link
Contributor

cargo --version

cargo 1.80.0 (376290515 2024-07-16)

@nokyan
Copy link
Owner Author

nokyan commented Jul 26, 2024

Hm. Could you send the full error?

@jojo2357
Copy link
Contributor

error[E0658]: use of unstable library feature 'lazy_cell'
  --> src/utils/settings.rs:12:43
   |
12 | pub static SETTINGS: LazyLock<Settings> = LazyLock::new(Settings::default);
   |                                           ^^^^^^^^^^^^^
   |
   = note: see issue #109736 <https://github.com/rust-lang/rust/issues/109736> for more information

For more information about this error, try `rustc --explain E0658`.
error: could not compile `resources` (lib) due to 125 previous errors

There are obviously a lot of repeated code usages that were omitted for brevity. Im using gnome builder and i tried cleaning, rming the build dir and still nothing

@nokyan
Copy link
Owner Author

nokyan commented Jul 26, 2024

error[E0658]: use of unstable library feature 'lazy_cell'
  --> src/utils/settings.rs:12:43
   |
12 | pub static SETTINGS: LazyLock<Settings> = LazyLock::new(Settings::default);
   |                                           ^^^^^^^^^^^^^
   |
   = note: see issue #109736 <https://github.com/rust-lang/rust/issues/109736> for more information

For more information about this error, try `rustc --explain E0658`.
error: could not compile `resources` (lib) due to 125 previous errors

There are obviously a lot of repeated code usages that were omitted for brevity. Im using gnome builder and i tried cleaning, rming the build dir and still nothing

I'm not sure but I think GNOME Builder might be using the Flatpak Rust SDK to build Flatpaks, maybe that's the cause. Have you updated org.freedesktop.Sdk.Extension.rust-stable?

@jojo2357
Copy link
Contributor

Yeah reinstalled SDK's and that did the trick.

Quick impression is that prints when a new app is detected and why that process was accepted under that .desktop (and maybe the path of that file if applicable)

@nokyan
Copy link
Owner Author

nokyan commented Jul 27, 2024

Quick impression is that prints when a new app is detected and why that process was accepted under that .desktop (and maybe the path of that file if applicable)

Added! I didn't include the path to the .desktop file in the prints for associating a process with an app because that would make those debug statements very verbose in my opinion, but the path to the desktop file is included in the initial app detection debug statements.

@jojo2357
Copy link
Contributor

While yes, this is exactly what I asked for, I had envisioned printing only when a new app (like a root process, not its children) was detected, since imo children should be accepted nearly automatically to their parent app.

Firefox gets spammy with all the children it spawns when at the end of the day it is all still ff.

Would it be possible to only print for new apps being detected?

I also noticed that executable name matching isn't helpful too much since it is hard to trace that down, could that be logged?

@nokyan
Copy link
Owner Author

nokyan commented Jul 27, 2024

I think only logging some processes could make the logs confusing, one would expect to be all processes logged in my opinion.

I also noticed that executable name matching isn't helpful too much since it is hard to trace that down, could that be logged?

Do you mean something like this?
image

@jojo2357
Copy link
Contributor

I see this

DEBUG resources::utils::app > Associating process 4086624 with app "Game 455712169795780630" (ID: discord-455712169795780630) based on process executable name matching with app executable name

@nokyan
Copy link
Owner Author

nokyan commented Jul 27, 2024

I see this

DEBUG resources::utils::app > Associating process 4086624 with app "Game 455712169795780630" (ID: discord-455712169795780630) based on process executable name matching with app executable name

Yes, my screenshot was just a quick local change. The executable name is printed at the beginning during the initial app detection though, is this really necessary then?

@jojo2357
Copy link
Contributor

Strictly, no, but for convenience, especially if the .desktop is quietly overwritten by another .desktop, could prove convenient imo

@nokyan
Copy link
Owner Author

nokyan commented Jul 27, 2024

Strictly, no, but for convenience, especially if the .desktop is quietly overwritten by another .desktop, could prove convenient imo

Ah, I see. I'll include it then.

@jojo2357
Copy link
Contributor

In fact, that overwriting is why my Dolphin install is not detected correctly ;)

@jojo2357
Copy link
Contributor

Could you also add the directories scanned for .desktops? Not strictly required, but helpful

@nokyan
Copy link
Owner Author

nokyan commented Jul 27, 2024

Could you also add the directories scanned for .desktops? Not strictly required, but helpful

Sort of like this?
DEBUG resources::utils::app > Using the following directories for app detection: ["/app/share/applications", "/usr/share/applications", "/usr/share/runtime/share/applications", "/run/host/user-share/applications", "/run/host/usr/share/applications", "/run/host/share/applications", "/var/lib/flatpak/exports/share/applications", "/home/nokyan/.local/share/flatpak/exports/share/applications", "/var/lib/snapd/desktop/applications", "/home/nokyan/.local/share/applications"]

@jojo2357
Copy link
Contributor

exactly. then I know why my .desktop isnt loaded because /usr/local/share isnt scanned

@nokyan
Copy link
Owner Author

nokyan commented Jul 27, 2024

Pushed. :)

@jojo2357
Copy link
Contributor

Looks good, now I can actually start to figure stuff out.

@nokyan nokyan marked this pull request as ready for review July 27, 2024 17:19
@jojo2357 jojo2357 mentioned this pull request Jul 27, 2024
@nokyan nokyan merged commit d771465 into main Jul 27, 2024
1 of 2 checks passed
@nokyan nokyan deleted the more-debug-statements branch July 27, 2024 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add debug prints for app detection
2 participants