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

prusa-slicer: 2.8.0 -> 2.9.0 #367376

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Conversation

me-and
Copy link
Contributor

@me-and me-and commented Dec 22, 2024

Update Prusa Slicer to the latest v2.9.0. This builds on @cbat98's as-yet unmerged PR for v2.8.1 in #358215.

Changelog:

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.

@cbat98
Copy link

cbat98 commented Dec 22, 2024

Thank you for taking a look at the 2.9.0 update :) Appreciate you building on my attempt at 2.8.1.

@me-and me-and marked this pull request as ready for review December 22, 2024 20:33
@me-and me-and added 8.has: package (update) This PR updates a package to a newer version 8.has: package (new) This PR adds a new package labels Dec 22, 2024
@jmickelin
Copy link

Nice stuff! I tested it and it works 👍

@maxammann raised a point in the previous PR that PrusaSlicer exhibits a bug with downloading for Curl versions newer than 7.88.1. Seeing how it's already broken in the current 2.8.0 package in nixpkgs, I think we can leave it as-is for this PR and then discuss overriding the Curl dependency in a followup PR.

@me-and
Copy link
Contributor Author

me-and commented Dec 23, 2024

I am getting a breakage in the super-slicer-beta package, which I've not yet debugged further, or even checked if it exists prior to this change...

CMake Error at src/occt_wrapper/CMakeLists.txt:22 (find_package):
  Could not find a configuration file for package "OpenCASCADE" that is
  compatible with requested version "7.6.2".

  The following configuration files were considered but not accepted:

    /nix/store/yqk4xxvw5pmjjq3ah4dahka5i8yxw0z6-opencascade-occt-7.6.1/lib/cmake/opencascade/OpenCASCADEConfig.cmake, version: 7.6.1



-- Configuring incomplete, errors occurred!

@jmickelin
Copy link

jmickelin commented Dec 23, 2024

The issue comes from the following line: https://github.com/supermerill/SuperSlicer/blob/master/src/occt_wrapper/CMakeLists.txt#L22

It's really strange that the super-slicer derivation is coupled to the prusa-slicer one like this, since the former is lagging behind a lot still. They really should be independent packages with their own build requirements these days.

In any case, since super-slicer hardcodes the dependency on OCCT-7.6.2 and prusa-slicer pins it to 7.6.1, I think you need to override it "back" in super-slicer.nix, something like this:

--- a/super-slicer.nix
+++ b/super-slicer.nix  
@@ -6,6 +6,7 @@
   wxGTK31,
   prusa-slicer,
   libspnav,
+  opencascade-occt_7_6
 }:
 let
   appname = "SuperSlicer";
@@ -129,7 +130,10 @@ let
       fetchSubmodules = true;
     };
   });
-  prusa-slicer-wxGTK-override = prusa-slicer.override { wxGTK-override = wxGTK31-prusa; };
+  prusa-slicer-dependencies-override = prusa-slicer.override { 
+    wxGTK-override = wxGTK31-prusa; 
+    opencascade-occt_7_6_1 = opencascade-occt_7_6;
+  };
   allVersions = builtins.mapAttrs (
-    _name: version: (prusa-slicer-wxGTK-override.overrideAttrs (override version))
+    _name: version: (prusa-slicer-dependencies-override.overrideAttrs (override version))
   ) versions;

It looks a bit hacky, with the argument opencascade-occt_7_6_1 being set to (effectively) opencascade-occt-7_6_2 but I think it's inevitable when it's coupled like this. The other option would be to patch SuperSlicer to downgrade the required version. Surely it would encounter the same bug that caused PrusaSlicer to pin it in the first place, so it could make sense: prusa3d/PrusaSlicer@c6a0210#diff-54d1bb7ea7d9b83fb3c587bc042046732a50bb843a067cd86c445bafbb6ba1eeR7

@cbat98
Copy link

cbat98 commented Dec 30, 2024

I don't know if it's something wacky with my setup, but to get it to build for me, I had to also apply this patch:

diff --git a/src/slic3r-arrange/include/arrange/DataStoreTraits.hpp b/src/slic3r-arrange/include/arrange/DataStoreTraits.hpp
index ba932d57a..624569d3f 100644
--- a/src/slic3r-arrange/include/arrange/DataStoreTraits.hpp
+++ b/src/slic3r-arrange/include/arrange/DataStoreTraits.hpp
@@ -71,7 +71,7 @@ template<class T, class TT = T> using WritableDataStoreOnly = std::enable_if_t<I
 template<class T, class ArrItem>
 void set_data(ArrItem &itm, const std::string &key, T &&data)
 {
-    WritableDataStoreTraits<ArrItem>::template set(itm, key, std::forward<T>(data));
+    WritableDataStoreTraits<ArrItem>::template set<T>(itm, key, std::forward<T>(data));
 }

template constexpr bool IsReadWritableDataStore = IsDataStore && IsWritableDataStore;

I'm not sure why this was needed, but I was seeing an error in the build at about 55%.

@me-and
Copy link
Contributor Author

me-and commented Dec 31, 2024

I tried to split out the super-slicer package so it no longer had the dependency on prusa-slicer, but I couldn't work out how to get that working. I think the complicating factor is the use of darwin.apple_sdk_11_0.callPackage in all-packages.nix, but in honesty I didn't care enough to spend a long time on it.

In any case, @jmickelin's approach worked for me, although I modified it slightly to emulate the existing override of wxGTK in super-slicer. I'll push that patch shortly.

@cbat98 I'm baffled that you're seeing that error when it's working fine for me building out of the nixpkgs tree. Possibly you have an overlay confusing things? How do you get on with nix-shell -p nixpkgs-review --run 'nixpkgs-review pr 367376', which I think enforces a clean environment?

@me-and
Copy link
Contributor Author

me-and commented Dec 31, 2024

...oh, that's odd. I'm now seeing a build failure as well, which looks like the same build failure as @cbat98 saw. I was building at the tip of my branch, but it looks like the build is broken by something else that's changed since I branched off...

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review


x86_64-linux

❌ 2 packages failed to build:
  • prusa-slicer
  • prusa-slicer.debug
✅ 5 packages built:
  • opencascade-occt_7_6_1
  • super-slicer (super-slicer-latest)
  • super-slicer-beta
  • super-slicer-beta.debug
  • super-slicer.debug (super-slicer-latest.debug)

@me-and me-and marked this pull request as draft December 31, 2024 23:37
@me-and
Copy link
Contributor Author

me-and commented Jan 3, 2025

...that was daft. I backed out 910f66b because I'd interpreted the error I saw to mean the patch wasn't necessary. Turns out it just means the file the patch was applying to was in a different file.

Assuming it doesn't find any other problems, I'll push an updated branch as soon as my local build has finished.

me-and added 2 commits January 3, 2025 19:09
When overriding the postPatch attribute from prusa-slicer, handle the
scenario where prusa-slicer doesn't have a postPatch attribute, as well
as the scenario where it does.
super-slicer is built by overriding prusa-slicer, and the updated
libraries as part of the update of prusa-slicer to v2.9.0 broke
super-slicer's pinned libraries.

Ideally we'd split off super-slicer as a separate package rather than
continuing to make changes in prusa-slicer then undo them in
super-slicer, but that seems to be a much more substantial job, so stick
with the simple approach for now.
@me-and
Copy link
Contributor Author

me-and commented Jan 3, 2025

There we go!

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review


x86_64-linux

✅ 7 packages built:
  • opencascade-occt_7_6_1
  • prusa-slicer
  • prusa-slicer.debug
  • super-slicer (super-slicer-latest)
  • super-slicer-beta
  • super-slicer-beta.debug
  • super-slicer.debug (super-slicer-latest.debug)

@me-and me-and marked this pull request as ready for review January 3, 2025 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 8.has: package (update) This PR updates a package to a newer version 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants