Package: guix-patches;
Reported by: nomike <nomike <at> nomike.com>
Date: Fri, 23 May 2025 02:02:02 UTC
Severity: normal
Tags: patch
Message #8 received at 78555 <at> debbugs.gnu.org (full text, mbox):
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> To: nomike <nomike <at> nomike.com> Cc: 78555 <at> debbugs.gnu.org Subject: Re: [bug#78555] [PATCH] gnu: Add pythonscad Date: Mon, 26 May 2025 21:21:54 +0900
Hi!
nomike <nomike <at> nomike.com> writes:
> * gnu/packages/engineering.scm (pythonscad): New variable
>
> Change-Id: I66737d0eaaa7546dcd407d7371166c9b5d5d9b31
> ---
> gnu/packages/engineering.scm | 77 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 75 insertions(+), 2 deletions(-)
>
> diff --git a/gnu/packages/engineering.scm b/gnu/packages/engineering.scm
> index 8b1332e186..c81f4d5dbe 100644
> --- a/gnu/packages/engineering.scm
> +++ b/gnu/packages/engineering.scm
> @@ -3315,6 +3315,79 @@ (define-public openscad
> (home-page "https://openscad.org/")
> (license license:gpl2+))))
>
> +(define-public pythonscad
> + (let ((commit "b856456fcf26e089a616e5a9b5d685b8a8f2e2c1")
> + (version "2025.05.21")
> + (revision "0"))
> + (package
> + (inherit openscad)
> + (name "pythonscad")
> + (version (git-version version revision commit))
> + (source
> + (origin
> + (method git-fetch)
> + (uri (git-reference
> + (url "https://github.com/pythonscad/pythonscad")
> + (commit commit)
> + (recursive? #t)))
Use of recursive? #t should be explained in a comment above; it's
typically used to bundle libraries that we'd rather package separately
and link to, if possible.
> + (sha256
> + (base32 "0bjpvj94m3kplpnmnlai7mjx45d5acnqw943w3p1mprg8wrp3ap6"))
> + (file-name (git-file-name name version))))
> + (arguments
> + (list
> + #:configure-flags
> + #~(list "-DCMAKE_BUILD_TYPE=Release"
> + "-DUSE_BUILTIN_OPENCSG=ON"
> + "-DMANIFOLD_PYBIND=OFF"
> + "-DMANIFOLD_TEST=OFF"
> + "-DENABLE_TESTS=OFF"
Why is the test suite disabled? This also needs an explanation. We
strive to run test suites in Guix.
> + "-DEXPERIMENTAL=ON"
> + "-DSNAPSHOT=ON"
Are these really needed here? In general, we stick to the default
configuration unless we have specific needs.
> + "-DENABLE_PYTHON=ON"
> + (string-append "-DPYTHON_VERSION="
> + (string-join (reverse (list-tail (reverse (string-split #$
> + (package-version
> + (this-package-input
> + "python"))
> + #\.))
> + 1))
> "."))
You can use #$(version-major+minor (package-version (this-package-input
"python"))), from (guix utils) instead. Please try to fit under 80
characters maximum width, which is in our coding style guidelines.
> + "-DUSE_BUILTIN_CLIPPER2=OFF"
> + (string-append "-DOPENSCAD_VERSION="
> + #$version)
> + (string-append "-DOPENSCAD_COMMIT="
> + #$commit)
> + "-DENABLE_EGL=ON"
> + "-DENABLE_GLX=ON")
> + #:phases
> + #~(modify-phases %standard-phases
> + (delete 'check)
> + (add-after 'unpack 'patch-source
> + (lambda* (#:key inputs #:allow-other-keys)
> + ;; <https://github.com/openscad/openscad/issues/5877>
It's very nice to have the cross-references to the issues being patched
here! It'd be even nicer if there was a short explanatory comment
introducing the issue, e.g. "The build system fails to find lib3mf; it
seems it should be using PC_LIB3MF_INCLUDEDIR instead of
PC_LIB3MF_INCLUDE_DIRS (see:
<https://github.com/openscad/openscad/issues/5880>)." It's more
verbose, but it's nice to summarize the issue to save the reader from
having to follow the link and read the whole conversation to figure it
out (perhaps they work offline too, or the link may disappear in the
future).
> + (substitute* "cmake/Modules/FindLib3MF.cmake"
> + (("PC_LIB3MF_INCLUDE_DIRS")
> + "PC_LIB3MF_INCLUDEDIR"))
> + (substitute* "CMakeLists.txt"
> + ;; <https://github.com/openscad/openscad/issues/5880>
> + (("target_link_libraries\\(OpenSCAD PRIVATE OpenGL::EGL\\)")
> + " find_package(ECM REQUIRED NO_MODULE)
> + list(APPEND CMAKE_MODULE_PATH ${ECM_MODULE_PATH})
> + find_package(EGL REQUIRED)
> + target_link_libraries(OpenSCAD PRIVATE EGL::EGL)")
> + ;; <https://github.com/openscad/openscad/issues/5897>
> + (("find_package\\(Nettle 3.4\\)")
> + "find_package(Nettle 3.4 REQUIRED)")
> + ;; Use the system sanitizers-cmake module.
> + (("\\$\\{CMAKE_SOURCE_DIR\\}/submodules/sanitizers-cmake/cmake")
> + (string-append (assoc-ref inputs "sanitizers-cmake")
> + "/share/sanitizers-cmake/cmake"))))))))
I'd use (search-input-directory (or native-inputs inputs)
"share/sanitizers-cmake/cmake")
The (or native-inputs inputs) is useful when cross-compiling; in that
situation, the native-inputs are in the native inputs, when doing a
straight build (host -> host), the native-inputs are fused into the
inputs.
> + (inputs (modify-inputs (package-inputs openscad)
> + (append libjpeg-turbo curl python nettle)))
> + (home-page "https://openscad.org/")
> + (description
> + "A fork of @code{OpenSCAD} that lets you use Python as its native language."))))
Perhaps you could reuse some of the original openscad package
description, to give more information on what this package does, and
append the above to specify how it differs from the original.
Could you please send a v2 with the above comments taken into
consideration?
--
Thanks,
Maxim
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.