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.7.4 -> 2.8.0 #325590

Merged
merged 1 commit into from
Aug 13, 2024
Merged

prusa-slicer: 2.7.4 -> 2.8.0 #325590

merged 1 commit into from
Aug 13, 2024

Conversation

Rose-David
Copy link
Contributor

@Rose-David Rose-David commented Jul 8, 2024

Description of changes

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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.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.

@Rose-David
Copy link
Contributor Author

Rose-David commented Jul 8, 2024

I'm trying to build it using nix-build -A prusa-slicer, but it fails with the following errors every time:

/nix/store/2p1sq1nr09xr3xb1a9lrjgdanvk1aakb-binutils-2.42/bin/ld: slic3r/liblibslic3r_gui.a(UserAccountCommunication.cpp.o): undefined reference to symbol 'EVP_DigestInit_ex@@OPENSSL_3.0.0'
/nix/store/2p1sq1nr09xr3xb1a9lrjgdanvk1aakb-binutils-2.42/bin/ld: /nix/store/jv2p6cmx23ihj5y4r98wnn2nmv4qhfh5-openssl-3.0.14/lib/libcrypto.so.3: error adding symbols: DSO missing from command line

I'm not sure what I am doing wrong.

@30350n
Copy link

30350n commented Jul 11, 2024

Sounds like it's this: PrusaSlicer #12884

Also, this would fix #322910.

@VuiMuich
Copy link
Contributor

VuiMuich commented Aug 1, 2024

Sounds like it's this: PrusaSlicer #12884

Also, this would fix #322910.

I tried the patch @mogorman posted in the linked upstream issue, but that didn't work for me. The linked gentoo patches though worked. so here they are are for reference:
Missing includes:

--- a/src/slic3r/Config/Version.cpp
+++ b/src/slic3r/Config/Version.cpp
@@ -7,6 +7,7 @@
 #include <cctype>
 
 #include <boost/filesystem/operations.hpp>
+#include <boost/filesystem/directory.hpp>
 #include <boost/nowide/fstream.hpp>
 
 #include "libslic3r/libslic3r.h"
--- a/src/slic3r/GUI/UserAccountCommunication.cpp
+++ b/src/slic3r/GUI/UserAccountCommunication.cpp
@@ -13,6 +13,7 @@
 #include <boost/filesystem.hpp>
 #include <boost/nowide/cstdio.hpp>
 #include <boost/nowide/fstream.hpp>
+#include <boost/nowide/convert.hpp>
 #include <curl/curl.h>
 #include <string>

and fixed linkin:

--- a/src/slic3r/CMakeLists.txt
+++ b/src/slic3r/CMakeLists.txt
@@ -376,6 +376,7 @@ set(SLIC3R_GUI_SOURCES
 )
 
 find_package(NanoSVG REQUIRED)
+find_package(OpenSSL REQUIRED)
 
 if (APPLE)
     list(APPEND SLIC3R_GUI_SOURCES
@@ -404,7 +405,7 @@ endforeach()
 
 encoding_check(libslic3r_gui)
 
-target_link_libraries(libslic3r_gui libslic3r avrdude libcereal imgui libvgcode GLEW::GLEW OpenGL::GL hidapi libcurl ${wxWidgets_LIBRARIES} NanoSVG::nanosvg NanoSVG::nanosvgrast)
+target_link_libraries(libslic3r_gui libslic3r avrdude libcereal imgui libvgcode GLEW::GLEW OpenGL::GL hidapi libcurl ${wxWidgets_LIBRARIES} NanoSVG::nanosvg NanoSVG::nanosvgrast OpenSSL::SSL OpenSSL::Crypto)
 
 if (MSVC)
     target_link_libraries(libslic3r_gui Setupapi.lib)

As a side note: linking used up a lot of memory (only have 8 gigs + 16GB swap) and froze my system once, so I built it with the --cores 3 flag and closed any other memory hungry programs.

Edit: just realized, that the patch from @mogorman is identical with the linking fix only, so I guess that's why it didn't build, becaus it was missing the includes still..

Edit 2: the patched package builds and runs perfectly fine on x86_64-linux

@K900
Copy link
Contributor

K900 commented Aug 1, 2024

The wxgtk branch should also be updated to match latest upstream. Also, we can cherry-pick the patch instead of waiting for it to be merged upstream.

@Rose-David
Copy link
Contributor Author

I'll go ahead and add the rest of the gentoo patches to my upstream PR, I didn't know if I needed both or just one.

@Rose-David
Copy link
Contributor Author

The upstream PR should work now.

@K900
Copy link
Contributor

K900 commented Aug 1, 2024

Also I'm not sure if this is actually a good idea but #331539 is actually how I ran into this PR in the first place.

@VuiMuich
Copy link
Contributor

VuiMuich commented Aug 1, 2024

I build it with all the previous and @K900's clang patch (actually only the part below) again using only --cores 3 and it was 1h21m23s whereas the before build took 2h7m55s...

@K900
Copy link
Contributor

K900 commented Aug 1, 2024

What hardware is that on? On my 7950X3D with --cores 32 (default) it takes, like, single digit minutes.

@VuiMuich
Copy link
Contributor

VuiMuich commented Aug 1, 2024

What hardware is that on? On my 7950X3D with --cores 32 (default) it takes, like, single digit minutes.

LENOVO ThinkPad X1 Carbon 2nd - Intel® Core™ i5-4300U CPU @ 1.90GHz (4) - 8GB RAM ☺️

@K900
Copy link
Contributor

K900 commented Aug 7, 2024

I've merged #331539, this will probably need a rebase now.

@Rose-David
Copy link
Contributor Author

@K900 I synced the fork, should I remove the "FIXME: remove in 2.8.0" code your PR added?

@K900
Copy link
Contributor

K900 commented Aug 9, 2024

Yes. Also, please rebase instead of merging.

@Rose-David
Copy link
Contributor Author

How do I do that? This is my first attempt at doing something like this, so I am still very new.

@K900
Copy link
Contributor

K900 commented Aug 9, 2024

It depends on how your nixpkgs remote is set up.

@Rose-David
Copy link
Contributor Author

My nixpkgs is set up on my computer with two remotes, origin (my nixpkgs fork), and upstream (upstream nixpkgs).

@K900
Copy link
Contributor

K900 commented Aug 9, 2024

Then git fetch upstream followed by git rebase -i upstream/master, apply your second commit as a fixup, then git push -f.

@Rose-David
Copy link
Contributor Author

Did I do it right? Just making sure.

@K900
Copy link
Contributor

K900 commented Aug 9, 2024

Mostly, but please change the commit message to the standard format (prusa-slicer: 2.7.4 -> 2.8.0), we have tooling that relies on that.

@Rose-David
Copy link
Contributor Author

Okay, if we're all set, should we cherry-pick the patch like you suggested (and how do I do that), or should we wait for it to be merged upstream (might take a while)

@K900
Copy link
Contributor

K900 commented Aug 9, 2024

Look into the patches attribute on derivations and the fetchpatch function.

@Rose-David
Copy link
Contributor Author

I think I figured it out! I got it to compile, and it runs!

@Rose-David
Copy link
Contributor Author

I guess that means it's time to make this a normal PR instead of a draft?

@K900
Copy link
Contributor

K900 commented Aug 9, 2024

Feel free to undraft this when you think it's OK to merge.

@Rose-David Rose-David marked this pull request as ready for review August 9, 2024 15:49
@Rose-David
Copy link
Contributor Author

Rose-David commented Aug 9, 2024

Update: the login feature doesn't work on my build. I tried to update wxwidgets to see if that was the issue, but it doesn't seem to want to compile afterward.

Nevermind, this appears to be an upstream issue. We will probably be waiting a while for that to be fixed, so we should be fine to merge.

@tko
Copy link

tko commented Aug 11, 2024

Result of nixpkgs-review pr 325590 run on aarch64-darwin 1

1 package failed to build:
  • prusa-slicer

The error looks to be the same as with 2.7.4 currently so not introduced by this PR:

no template named 'binary_function' in namespace 'boost::functional::detail'; did you mean '__binary_function'?
error: builder for '/nix/store/kg16943n1fm1yap9mh01hvma41ba6q1c-prusa-slicer-2.8.0.drv' failed with exit code 2;
       last 25 log lines:
       > include/boost/functional.hpp:45:24: note: '__unary_function' declared here
       >             using std::unary_function;
       >                        ^
       > include/boost/functional.hpp:487:68: error: no template named 'binary_function' in namespace 'boost::functional::detail'; did you mean '__binary_function'?
       >     class const_mem_fun1_ref_t : public boost::functional::detail::binary_function
       >                                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~^
       > include/boost/functional.hpp:46:24: note: '__binary_function' declared here
       >             using std::binary_function;
       >                        ^
       > include/boost/functional.hpp:533:73: error: no template named 'unary_function' in namespace 'boost::functional::detail'; did you mean '__unary_function'?
       >     class pointer_to_unary_function : public boost::functional::detail::unary_function
       >                                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~^
       > include/boost/functional.hpp:45:24: note: '__unary_function' declared here
       >             using std::unary_function;
       >                        ^
       > include/boost/functional.hpp:557:74: error: no template named 'binary_function' in namespace 'boost::functional::detail'; did you mean '__binary_function'?
       >     class pointer_to_binary_function : public boost::functional::detail::binary_function
       >                                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~^
       > include/boost/functional.hpp:46:24: note: '__binary_function' declared here
       >             using std::binary_function;
       >                        ^
       > 16 errors generated.
       > make[2]: *** [src/libslic3r/CMakeFiles/libslic3r.dir/build.make:77: src/libslic3r/CMakeFiles/libslic3r.dir/cmake_pch.hxx.pch] Error 1
       > make[1]: *** [CMakeFiles/Makefile2:1078: src/libslic3r/CMakeFiles/libslic3r.dir/all] Error 2
       > make: *** [Makefile:146: all] Error 2

@30350n
Copy link

30350n commented Aug 11, 2024

Update: the login feature doesn't work on my build. I tried to update wxwidgets to see if that was the issue, but it doesn't seem to want to compile afterward.

Nevermind, this appears to be an upstream issue. We will probably be waiting a while for that to be fixed, so we should be fine to merge.

Could this be related to this?

IMPORTANT note for Linux users

PrusaSlicer now depends on WebKit library, which greatly complicates its distribution. Latest Linux distributions (such as Ubuntu 24.04, Fedora 40) ship with newer version of WebKit than older (but still supported) distros. Bundling WebKit into the AppImage is difficult because it leads to various issues with its dependencies. So far we did not manage to create an AppImage that would reliably run on all relevant distros.

I don't see webkit being added as a dependency anywhere in here.

@Rose-David
Copy link
Contributor Author

Could this be related to this?

I don't think so, given that people are even experiencing this issue on windows.

I don't see webkit being added as a dependency anywhere in here.

That's my bad, but I tried all three versions of webkitgtk that are available on nixpkgs, and none of them changed the outcome.

There's an invisible "Connection Failed" message, (see my comment on PrusaSlicer Issue 12969) but that's all.

@30350n
Copy link

30350n commented Aug 12, 2024

There's an invisible "Connection Failed" message.

Yeah I got that too, but before that I also shortly got a message telling me to install glib-networking. So I added that to the buildInputs and now it's working.

Still not super sure how webkit is being linked exactly, but this seems to do the trick.

@Rose-David
Copy link
Contributor Author

Still not super sure how webkit is being linked exactly, but this seems to do the trick.

Interesting, I'll try this when I get home and add it to the PR.

@Rose-David
Copy link
Contributor Author

Sure enough, it works. No idea how its working without webkit, but we take those lol. I'll make sure to forward this fix to the folks having the same problem on the prusa repo.

@K900 K900 merged commit dc30d31 into NixOS:master Aug 13, 2024
25 of 26 checks passed
@Rose-David Rose-David deleted the prusa-slicer-2_8 branch August 13, 2024 17:05
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.

5 participants