[bug#50052] Fwd: Re: [bug#50052] [PATCH] Add prusa-slicer

[bug#50052] Fwd: Re: [bug#50052] [PATCH] Add prusa-slicer

[Daniel Trujillo Viedma]

Sorry, forgot to include debbugs...


El 15/8/21 a las 13:02, Xinglu Chen escribió:

> On Sat, Aug 14 2021, Daniel Trujillo Viedma wrote:
>
>> Hi! I'm Daniel Trujillo,
>>
>>
>> This is my first-time-ever contribution to GNU Guix, so please, don't
>> hold any nitpick to yourself!! :)
>>
>>
>> I don't know if these kind of packages are of interest, but this is
>> PrusaSlicer [0], a software to prepare 3D printings (hence I put it in
>> engineering.scm).
>>
>> The packaged version, 2.3.3 is the latest stable available at this
>> moment. But this version has as important problem [1]: It cannot be
>> invoked just naming the executable (through $PATH), because then, it
>> can't locate the resources directory. I learned **the hard way** that
>> the fix was applied *after* the release of the 2.3.3 version, so I
>> decided to backport the fix adding a patch because it's a show-stopper
>> to have to type the path to the executable in /gnu/store/... That's the
>> reason why the patch attached to this email contains, not only the
>> additions to engineering.scm, but also a patch that implements the
>> solution in the version 2.3.3 codebase (It's quite simple, it uses a
>> boost function to determine the path to the executable rather than
>> relying on argv[0]).
>>
>>
>> Known improvable things:
>> * It's configured to use GTK3. After many attempts to compile it under
>> GTK2 in a guix environment, sometimes it detected GTK right away, and
>> some ether times I had to add more includes. It wasn't quite reliable
>> and, according to the convention followed in gtk+ packages, probably it
>> would be better that prusa-slicer uses GTK3, and a hypothetical future
>> GTK2 version would be called prusa-slicer-gtk2.
>>
>> * In order for the above $PATH issue fix to work, it's crucial that the
>> cmake variable SLIC3R_FHS is set to off. This is the default value
>> according to the CMakeLists.txt, but because it's so important, probably
>> it should have been included in the configure-flags argument? Just for
>> clarity.
> That’s probably a good idea, in case upstream changes the default value
> in the future.
Great point, I'm doing it!
>
>> * Currently, the version displayed in the title bar is
>> "....2.3.3+UNKNOWN". This is because of another cmake option not set. It
>> doesn't have any influence in the software, as far as I know, but it's
>> arguably ugly. The version I currently use, doesn't display that in the
>> title bar, but it does in the "About" window, and it says
>> ".....2.3.3+linux-64". Maybe something like "GNU Guix" would be
>> prettier. But if I have to include the arch, I would have to dig deeper
>> into package definitions ^_^'''
>>
>>
>> I hope everything is in order, I'm looking forward to see your comments,
>> and hope I can start contributing more packages to Guix!!
>>
>>
>> Cheers,
>>
>> Dani.
>>
>>
>> [0] https://www.prusa3d.es/prusaslicer/
>>
>> [1] https://github.com/prusa3d/PrusaSlicer/issues/5542
>>
>>
>> From 522c1904cf62afac25a9d974091211adac760c25 Mon Sep 17 00:00:00 2001
>> From: Daniel Trujillo Viedma <danihacker.viedma@gmail.com>
>> Date: Sat, 14 Aug 2021 00:00:55 +0200
>> Subject: [PATCH] Add prusa-slicer
>>
>> ---
> The commit message should be in the GNU ChangeLog format, see the commit
> log for examples, or check the manual.
>
> <https://www.gnu.org/prep/standards/standards.html#Change-Logs>
Sorry!!! I have yet to be familiarized with that format, I'll see this 
article and examples before submitting the fixes, thanks for notice!
>> gnu/packages/engineering.scm | 58 ++++++++++++++++++++++
>> .../patches/prusa-slicer-backport-fix-5542.patch | 31 ++++++++++++
> This patch should also be added to the gnu/local.mk file.
Thanks for noticing, it will be added in the next submission.
>
>> 2 files changed, 89 insertions(+)
>> create mode 100644 
>> gnu/packages/patches/prusa-slicer-backport-fix-5542.patch
>>
>> diff --git a/gnu/packages/engineering.scm b/gnu/packages/engineering.scm
>> index 33c124a2ea..047d99c0af 100644
>> --- a/gnu/packages/engineering.scm
>> +++ b/gnu/packages/engineering.scm
>> @@ -2863,3 +2863,61 @@ for hooking Linux system calls in user space. 
>> This is achieved by
>> hot-patching the machine code of the standard C library in the memory of
>> a process.")
>> (license license:bsd-2))))
>> +
>> +(define-public prusa-slicer
>> + (package
>> + (name "prusa-slicer")
>> + (version "2.3.3")
>> + (source
>> + (origin
>> + (method git-fetch)
>> + (uri (git-reference
>> + (url "https://github.com/prusa3d/PrusaSlicer")
>> + (commit (string-append "version_" version))))
> The ‘file-name’ field should be set to
>
> (file-name (git-file-name name version))
>
> to give the git checkout, in the Guix store, a more descriptive name,
> otherwise its name is
>
> /gnu/store/hc1slayzjz8xrw24q8wzgcs6xrzfk74q-git-checkout
>
> which doesn’t really say much. This usually only applies if some kind
> of VCS repository is used, and not if ‘url-fetch’ is used.
I thought the checkout directory was temporary, but yes, even in the 
"origin reference" section of the Manual it says that it's a good idea 
to set it up when working with VCS. Added!
>
>> + (sha256
>> + (base32 "0w0synqi3iz9aigsgv6x1c6sg123fasbx19h4w3ic1l48r8qmpwm"))
>> + (patches (search-patches "prusa-slicer-backport-fix-5542.patch"))))
>> + (build-system cmake-build-system)
>> + (arguments
>> + `(#:configure-flags `("-DSLIC3R_GTK=3")))
>> + (native-inputs
>> + `(("pkg-config" ,pkg-config)))
>> + (inputs
>> + `(("perl" ,perl)
>> + ("glibc-locales" ,glibc-locales)
>> + ("adwaita-icon-theme" ,adwaita-icon-theme)
>> + ("boost" ,boost)
>> + ("tbb" ,tbb)
>> + ("curl" ,curl)
>> + ("zlib" ,zlib)
>> + ("eigen" ,eigen)
>> + ("expat" ,expat)
>> + ("libpng" ,libpng)
>> + ("mesa" ,mesa)
>> + ("glew" ,glew)
>> + ("cereal" ,cereal)
>> + ("nlopt" ,nlopt)
>> + ("openvdb" ,openvdb)
>> + ("cgal" ,cgal)
>> + ("gmp" ,gmp)
>> + ("mpfr" ,mpfr)
>> + ("qhull" ,qhull)
>> + ("wxwidgets" ,wxwidgets-3.1)
>> + ("coreutils" ,coreutils)
>> + ("grep" ,grep)
>> + ("sed" ,sed)
>> + ("glibc" ,glibc)
> These packages are already included in the ‘cmake-build-system’ by
> default, no need to add them explicitly.
Yes, I've verified that the package still builds successfully and the 
program launches. Thanks!
>
>> + ("gtk+" ,gtk+)
>> + ("pango" ,pango)
>> + ("dbus" ,dbus)))
>> + (home-page "https://www.prusa3d.com/prusaslicer/")
>> + (synopsis "G-code generator for 3D printers (RepRap, Makerbot, 
>> Ultimaker etc.)")
>> + (description
>> + "PrusaSlicer (formerly known as Slic3r Prusa Edition or Slic3r PE) is
>> +our own in-house developed slicer software based on the open-source 
>> project
>> +Slic3r. PrusaSlicer is an open-source, feature-rich, frequently updated
>> +tool that contains everything you need to export the perfect print files
> Nit: “perfect” sounds a bit too much like marketing speak; instead of
> “to export the perfect print files”, I suggest “to export to print
> files”.
>
> WDYT?
Absolutely agree. "Perfect" slicer is far from the state-of-the-art. I 
took this description from their web page, but if we can adapt it to be 
more objective, I would say: "PrusaSlicer (formerly known as Slic3r 
Prusa Edition or Slic3r PE) is a slicer software based on the 
open-source project Slic3r. PrusaSlicer is an open-source, feature-rich, 
frequently updated tool that contains everything you need to export the 
print files for your 3D printer.". Is this OK?
>
>> +for your Original Prusa 3D printer.")
>> + (license license:agpl3)))
> According the the LICENSE file, this should be agpl3+ (grep for “any
> later version”).
>
> <https://github.com/prusa3d/PrusaSlicer/blob/master/LICENSE>
I'm not sure. Probably you guys know more than me about licenses, but it 
seems to me that this is only AGPL3, because if I `rgrep "any later 
version"` the entire source code tree, I get 128 hits, mostly from 
dependencies. And the 3 matches from the LICENSE file are on a 
conditional voice (""If the Program specifies that a certain numbered 
version [...] "or any later version" applies to it, you have the option 
.......""), and the last match is just the instructions that says to put 
the notice at the start of every source code file (which I think they 
didn't do. rgrep "GNU Affero General Public" gives me 57 coincidences, 
mostly from resources, and no one of them mentions "or any later 
version", so I think it's only AGPL-3).
>
>
> I didn’t manage to build the package because my machine ran out of
> memory, but I assume that it built fine for you. :-)

Yes, I have suffered this also. To build this in a 16GB RAM machine I 
had to low the number of cores to 2 (guix build -M1 -c2 prusa-slicer). 
This prevents more than 2 programs running at the same time, and the 
memory doesn't get full. I think the hardest part is at the end of the 
process, when the GUI executable is being linked.


I've seen in some packages that there's a comment with the package size. 
Maybe I'll do the same as a warning that the build process can take up 
all the RAM available easily.

Thank you for your feedback!