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

Add Plasma Mobile Gear 21.05 #125156

Merged
merged 18 commits into from
Oct 6, 2021

Conversation

samueldr
Copy link
Member

@samueldr samueldr commented Jun 1, 2021

Motivation for this change

#121345 is a mess to review because it is just plain too big. I'll rebase #121345 on top of this PR, which will make it hold only the Plasma Mobile shell. (The "phone GUI environment".)

Those applications are independent from actual plasma mobile.

They can seemingly be used (with caveats) on e.g. Phosh.

Only applications with a release tags are packaged.

Note that these applications launch just right on my X11 desktop with Awesome WM. No real caveats there AFAICT.

✔️ This PR is ready for reviewing.


Caveats

None of those caveats are bugs in the packaging of these applications.

(Long scary list, not actual packaging bugs)

Icons wont work as expected

{
  # Work around issues with plasma mobile apps on phosh
  #  - https://bugs.kde.org/show_bug.cgi?id=435236
  hacks.mergedIcons = {
    enable = true;

    # Helps with breeze fallback icons under gnome/phosh
    # Otherwise it might use Adwaita
    removeLegacyIcons = true;
    disabledThemes = [
      "Adwaita"
    ];

    # Options that aren't used
    #masqueradeAs = [
    #  "Adwaita" # Doesn't help really
    #];
    #iconThemes = [
    #];
  };
}

Will not start by default

By default the apps on Phosh (or GNOME too I suppose?) won't start:

Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland ro tun on Wayland anyway.

They'll try to start using xcb/X11 and crash. How rude.

The workaround is to force Wayland... Rude...

{
  environment.variables = {
    QT_QPA_PLATFORM = "wayland";
  };
}

It is unclear why those apps are willfully ignoring starting as wayland apps in Phosh (and GNOME I suppose).

CSDs in Phosh

While not really an issue, one can completely disable Qt CSDs this way:

{
  environment.variables = {
    QT_WAYLAND_DISABLE_WINDOWDECORATION = "1";
  };
}

Seems to me there is a need for some coordination between "phone environments" to properly tell when CSDs are needed and when they're not for docked/undocked modes :/.

Things done

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Added a release notes entry if the change is major or breaking
  • Fits CONTRIBUTING.md.

Actual tests:

I ran applications on my X11 desktop and under Phosh. Apps were also ran in their previous releases under Plasma Mobile, but not this most up-to-date branch yet.

Fully tested:

  • alligator
  • calindori
  • kalk
  • kclock
  • koko
  • krecorder
  • ktrip
  • kweather (not part of 21.05)
  • plasma-camera (not part of 21.05)
  • plasma-phonebook

Features untested:

  • plasma-dialer (dialing/receiving a call... basically ofono integration)
  • spacebar (sms receiving/sending... basically ofono integration)

cc @NixOS/qt-kde

TODO

  • Use an update script for Plasma Mobile Gear

@r-rmcgibbo
Copy link

Result of nixpkgs-review pr 125156 at 605cb65f run on x86_64-linux 1

2 packages skipped due to time constraints:
  • kclock
  • kweather
10 packages built successfully:
  • alligator
  • calindori
  • kalk
  • kirigami-addons
  • krecorder
  • libqofono
  • plasma-camera
  • plasma-dialer
  • plasma-phonebook
  • spacebar-sms
3 suggestions:
  • warning: missing-patch-comment

    Consider adding a comment explaining the purpose of this patch on the line preceeding.
    Near pkgs/tools/networking/libqofono/default.nix:28:5:

       |
    28 |     ./0001-NixOS-Skip-tests-they-re-shock-full-of-hardcoded-FHS.patch
       |     ^
    
  • warning: missing-patch-comment

    Consider adding a comment explaining the purpose of this patch on the line preceeding.
    Near pkgs/tools/networking/libqofono/default.nix:24:5:

       |
    24 |     (substituteAll {
       |     ^
    
  • warning: unused-argument

    Unused argument: fetchpatch.
    Near pkgs/applications/science/math/kalk/default.nix:4:3:

      |
    4 | , fetchpatch
      |   ^
    

@samueldr samueldr force-pushed the feature/plasma-mobile-apps-only branch from 605cb65 to 3140785 Compare June 1, 2021 04:53
@samueldr samueldr force-pushed the feature/plasma-mobile-apps-only branch 2 times, most recently from 0f36d6b to b06212c Compare June 1, 2021 21:00
@samueldr
Copy link
Member Author

samueldr commented Jun 6, 2021

I intend to merge this by the end of this week. (Unless someone else merges this beforehand.)

pkgs/applications/graphics/plasma-camera/default.nix Outdated Show resolved Hide resolved
pkgs/tools/networking/libqofono/default.nix Outdated Show resolved Hide resolved
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
@dotlambda
Copy link
Member

Running kclock prints

QML debugging is enabled. Only use this in a safe environment.

Is there a way to make a release build without that?

@samueldr samueldr force-pushed the feature/plasma-mobile-apps-only branch 3 times, most recently from 22bfa81 to d63ce71 Compare June 7, 2021 20:54
@samueldr
Copy link
Member Author

samueldr commented Jun 7, 2021

@samueldr samueldr force-pushed the feature/plasma-mobile-apps-only branch from d63ce71 to 00caaee Compare June 7, 2021 21:09
@samueldr
Copy link
Member Author

samueldr commented Jun 7, 2021

I believe everything @dotlambda requested has been changed.

About inclusion in pkgs/top-level/qt5-packages.nix... Really it just wasn't clear to me where things should go for a lot of this.

@dotlambda
Copy link
Member

About inclusion in pkgs/top-level/qt5-packages.nix... Really it just wasn't clear to me where things should go for a lot of this.

I would say: everything that might be used with multiple versions of Qt, i.e. libraries, go in qt5-packages.nix.

@samueldr

This comment has been minimized.

@samueldr samueldr self-assigned this Jun 10, 2021
@samueldr samueldr marked this pull request as draft June 10, 2021 18:22
@samueldr samueldr removed their assignment Jun 10, 2021
@samueldr samueldr force-pushed the feature/plasma-mobile-apps-only branch 2 times, most recently from 9eb31ed to a49cf97 Compare July 14, 2021 00:47
@samueldr
Copy link
Member Author

samueldr commented Jul 14, 2021

Previous force pushes:

  • Strictly rebases on top of nixos-unstable
  • Drops a conflicting alias we definitely can drop (kalk -> kalker #125555)

@samueldr samueldr force-pushed the feature/plasma-mobile-apps-only branch from ada3cdc to aa4b9b5 Compare September 27, 2021 20:06
@samueldr
Copy link
Member Author

Last force push to work around a bogus check barking angrily...

@samueldr samueldr force-pushed the feature/plasma-mobile-apps-only branch from aa4b9b5 to 0dbe821 Compare September 29, 2021 21:15
@Mindavi
Copy link
Contributor

Mindavi commented Oct 2, 2021

I tried this out on the pinephone (tested kclock, krecorder and kalk).
IMG_20211002_120413

Apart from krecorder missing some icons for the buttons (I had to click on them blindly), they do build on aarch64. Krecorder somehow doesn't show recordings that I save, but it's probably not related to packaging.

Copy link
Contributor

@Mindavi Mindavi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quickly skimmed over, LGTM

@samueldr
Copy link
Member Author

samueldr commented Oct 2, 2021

@Mindavi just so it's clear, there's a list of caveats, and part of it has a Icons wont work as expected description. Icons missing outside of Plasma environments is a known issue, and squarely an (ignored) upstream issue.

@@ -0,0 +1 @@
WGET_ARGS=( http://download.kde.org/stable/plasma-mobile/21.05 -A '*.tar.xz' )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's strange... Here's what I know: wget is acting as a (not very smart) link scraper, looking for <a> tags with filenames that match the given pattern. This has broken pretty spectacularly in the past when, for example, KDE modified their website to insert most of the page content with JavaScript. I wonder if, at some point, the page source contained links for both versions? It only seems to have 21.05 now, though.

The alias has been present for 40 days, the previous package was present
for not more than 3 days with the `kalk` name.
Used similarly to the KDE Gear scripts:

```
 $ ./maintainers/scripts/fetch-kde-qt.sh pkgs/applications/plasma-mobile/
```
Generated with the fetch script.
Though it is empty, the package set is valid. Next changes introduce
derivations for packages we maintain.
requires data that is not versioned:

 - https://invent.kde.org/graphics/koko#packaging

Snapshotted because... Obviously that won't do with Nix.
@samueldr samueldr force-pushed the feature/plasma-mobile-apps-only branch from 0dbe821 to 8a23358 Compare October 6, 2021 03:21
@samueldr samueldr merged commit 3128520 into NixOS:master Oct 6, 2021
@samueldr samueldr deleted the feature/plasma-mobile-apps-only branch October 6, 2021 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants