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

cura-appimage: init at 5.9.0 #372614

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

pbek
Copy link
Contributor

@pbek pbek commented Jan 10, 2025

I took over #367409 (because there was no response anymore), matched the by-name path and added the desktop icon.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@pbek pbek mentioned this pull request Jan 10, 2025
13 tasks
@pbek
Copy link
Contributor Author

pbek commented Jan 10, 2025

@bct, @FliegendeWurst, @nh2, you are welcome to test.

@pbek pbek force-pushed the feature/cura-appimage branch from 4682f45 to 4eb66b1 Compare January 10, 2025 10:21
@pbek
Copy link
Contributor Author

pbek commented Jan 10, 2025

Thank you, @FliegendeWurst!

@pbek pbek force-pushed the feature/cura-appimage branch from 4eb66b1 to 1e35f80 Compare January 10, 2025 11:34
@pbek pbek mentioned this pull request Jan 10, 2025
2 tasks
@pbek pbek force-pushed the feature/cura-appimage branch from 1e35f80 to 8bc311d Compare January 10, 2025 11:52
@pbek
Copy link
Contributor Author

pbek commented Jan 10, 2025

@bct, you are not a nixpkgs maintainer yet! You need to add yourself to the nixpkgs maintainer list first.

@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Jan 10, 2025
@shimunn
Copy link
Contributor

shimunn commented Jan 11, 2025

meta.mainProgram should be set to cura

@FliegendeWurst
Copy link
Member

To get automatic updates

diff --git a/pkgs/by-name/cu/cura-appimage/package.nix b/pkgs/by-name/cu/cura-appimage/package.nix
index c5b4f16297c9..8957584b3163 100644
--- a/pkgs/by-name/cu/cura-appimage/package.nix
+++ b/pkgs/by-name/cu/cura-appimage/package.nix
@@ -7,9 +7,10 @@
   appimageTools,
   copyDesktopItems,
   makeDesktopItem,
+  nix-update-script,
 }:
 
-let
+stdenvNoCC.mkDerivation rec {
   pname = "cura-appimage";
   version = "5.9.0";
 
@@ -52,9 +53,7 @@ let
     done
     QT_QPA_PLATFORM=xcb exec "${curaAppimageToolsWrapped}/bin/${appimageBinName}" "''${args[@]}"
   '';
-in
-stdenvNoCC.mkDerivation rec {
-  inherit pname version;
+
   dontUnpack = true;
 
   nativeBuildInputs = [ copyDesktopItems ];
@@ -107,6 +106,8 @@ stdenvNoCC.mkDerivation rec {
     runHook postInstall
   '';
 
+  passthru.updateScript = nix-update-script { extraArgs = [ "--version-regex=([56789].+)" ]; };
+
   meta = {
     description = "3D printing software";
     homepage = "https://github.com/ultimaker/cura";

@pbek pbek force-pushed the feature/cura-appimage branch from 1eed196 to 5b2a71e Compare January 11, 2025 11:39
@pbek
Copy link
Contributor Author

pbek commented Jan 11, 2025

Thank you, @shimunn and @FliegendeWurst.

@shimunn
Copy link
Contributor

shimunn commented Jan 11, 2025

Can't open an file:

nix run github:pbek/nixpkgs/feature/cura-appimage#cura-appimage --refresh

crashes with the following terminal output, when clikcing on File -> Open file(s):

qt.qml.typeresolution.cycle: Cyclic dependency detected between "file:///nix/store/32iqlcylrd8m0vyifrjll41ni3inz4mr-cura-appimage-tools-output-5.9.0-extracted/share/cura/resources/qml/Actions.qml" and "file:///nix/store/32iqlcylrd8m0vyifrjll41ni3inz4mr-cura-appimage-tools-output-5.9.0-extracted/share/cura/resources/qml/Actions.qml"

@FliegendeWurst
Copy link
Member

I get the same error, but I can open files just fine. I set GTK_USE_PORTAL=1 which might be related.

@shimunn
Copy link
Contributor

shimunn commented Jan 11, 2025

I get the same error, but I can open files just fine. I set GTK_USE_PORTAL=1 which might be related.

The is an similar recommendation in an Ultimaker forum post:

-platformtheme gtk3

Which works for me

nix run github:pbek/nixpkgs/feature/cura-appimage#cura-appimage --refresh -- -platformtheme gtk3

If this flag is required for Cura to run on NixOs maybe we should set it via makeWrapper ?

@pbek
Copy link
Contributor Author

pbek commented Jan 12, 2025

nix run github:pbek/nixpkgs/feature/cura-appimage#cura-appimage --refresh

I had no issues running this on another machine (KDE Plasma) and opening a file...

qt.qml.typeresolution.cycle: Cyclic dependency detected between "file:///nix/store/32iqlcylrd8m0vyifrjll41ni3inz4mr-cura-appimage-tools-output-5.9.0-extracted/share/cura/resources/qml/Actions.qml" and "file:///nix/store/32iqlcylrd8m0vyifrjll41ni3inz4mr-cura-appimage-tools-output-5.9.0-extracted/share/cura/resources/qml/Actions.qml"

But I also get this log output. No crash.

@pbek
Copy link
Contributor Author

pbek commented Jan 12, 2025

Using it with -platformtheme gtk3 gives me a GTK file picker under Plasma 6, instead of the native KDE one. So I guess this isn't something we should change in the package. 🤔

@pbek
Copy link
Contributor Author

pbek commented Jan 12, 2025

Wow, you can even have the opposite crash with Qt apps. I tried qownnotes -- -platformtheme gtk3 (QWidget app) and it crashed when opening a file with:

(qownnotes:507484): GLib-GIO-ERROR **: 07:07:03.086: Settings schema 'org.gtk.Settings.FileChooser' is not installed
fish: Job 1, 'qownnotes -- -platformtheme gtk3' terminated by signal SIGTRAP (Trace or breakpoint trap)

@FliegendeWurst
Copy link
Member

This sort of crash might be fixed by #271037 eventually. For now we probably need the gappsWrapper

@FliegendeWurst
Copy link
Member

Using it with -platformtheme gtk3 gives me a GTK file picker under Plasma 6, instead of the native KDE one. So I guess this isn't something we should change in the package. 🤔

I think if you also set GTK_USE_PORTAL=1, you will get the KDE one. Though setting that variable is not recommended by upstream.

@pbek pbek force-pushed the feature/cura-appimage branch from 5b2a71e to 58f8bd1 Compare January 12, 2025 13:14
@pbek
Copy link
Contributor Author

pbek commented Jan 12, 2025

I added GTK_USE_PORTAL=1, it doesn't seem to break the file open dialogs under KDE Plasma.

Copy link
Member

@FliegendeWurst FliegendeWurst left a comment

Choose a reason for hiding this comment

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

Works for me, Plasma X11.

Would be good to test on a GTK-based desktop too.

@pbek
Copy link
Contributor Author

pbek commented Jan 12, 2025

Works for me, Plasma X11.

Great, I tested on Plasma Wayland.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jan 12, 2025
@pbek
Copy link
Contributor Author

pbek commented Jan 14, 2025

@shimunn, do you have a GTK based desktop?

@shimunn
Copy link
Contributor

shimunn commented Jan 14, 2025

@shimunn, do you have a GTK based desktop?

I dont think so, I'm using i3

@pbek
Copy link
Contributor Author

pbek commented Jan 14, 2025

Do you still have the crash?

@shimunn
Copy link
Contributor

shimunn commented Jan 15, 2025

Do you still have the crash?

Yes

@pbek pbek force-pushed the feature/cura-appimage branch from 05d8900 to c4f9b23 Compare January 16, 2025 09:40
@pbek
Copy link
Contributor Author

pbek commented Jan 16, 2025

@shimunn, can you please try again?

@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jan 16, 2025
@pbek
Copy link
Contributor Author

pbek commented Jan 21, 2025

Anything missing here?

@pbek
Copy link
Contributor Author

pbek commented Jan 21, 2025

@ofborg test cura-appimage

@pbek pbek force-pushed the feature/cura-appimage branch from d2898c2 to 3f76940 Compare January 22, 2025 18:33
@pbek
Copy link
Contributor Author

pbek commented Jan 22, 2025

All done now.

Signed-off-by: Patrizio Bekerle <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants