Message ID | fMPwP_w5oQHBqZA65obsAuwNMmmgwtFFPJLQf6FCLu_sIEvSg_Y8lUxQHXxhrODfLfTMMoFzgtKdPx8P6jEOaw9hf1yCnwVF5IKSngelxMo=@elenq.tech |
---|---|
State | Accepted |
Headers | show |
Series | [bug#45573] Correct freecad runtime errors | expand |
Context | Check | Description |
---|---|---|
cbaines/comparison | success | View comparision |
cbaines/git branch | success | View Git branch |
cbaines/applying patch | fail | View Laminar job |
cbaines/issue | success | View issue |
On Thu, Dec 31, 2020 at 06:47:59PM +0000, Ekaitz Zarraga wrote: > I attach 4 patches that correct the runtime issues freecad has and also supply some packages needed as a dependency. > > Freecad is a very complex package that is hard to build because many of its dependencies' buildsystem is broken. The approach followed here is the same that Nix follows and appears to work correctly. > > The changes correct the Draft module, which wasn't available because pivy was not added as a dependency. > > For context, see this message: > https://lists.gnu.org/archive/html/guix-devel/2020-12/msg00123.html Thanks! Here is my feedback: > Subject: [PATCH 1/4] gnu: Add coin3D-4. > > * gnu/packages/graphics.scm (coin3D-4): New variable. > + (snippet > + '(begin > + (for-each delete-file > + '("cfg/csubst.exe" > + "cfg/wrapmsvc.exe")) Please add a brief code comment like "Delete binaries". > + (substitute* "CMakeLists.txt" > + ((".*cpack.d.*") "")) > + #t)))) What does this do? Please add an explanatory comment. > Subject: [PATCH 3/4] gnu: Add python-pivy. > > * gnu/packages/python-xyz.scm (python-pivy): New variable. > + (snippet > + '(begin > + (substitute* "CMakeLists.txt" > + (("\\$\\{SoQt_INCLUDE_DIRS}") > + "${Coin_INCLUDE_DIR};${SoQt_INCLUDE_DIRS}")) > + #t)))) Origin snippets affect what is returned by `guix build --source python-pivy`, and are used for correcting very serious bugs in the source code or for fixing so-called "freedom issues", such as removing pre-compiled binaries. We prefer to make other types of changes in custom build phases in a package's arguments. I'm not sure exactly what this substitution does — please add a comment — but perhaps it would be more appropriate in a custom build phase? > + (arguments > + `(#:tests? #f)) Why are the tests disabled? We aim to make Guix packages pass upstream test suites, so there should be a reason for skipping them. If there is no test suite, just add a comment saying so. Same question about the soqt package. > Subject: [PATCH 4/4] gnu: freecad correct runtime errors > > * gnu/packages/engineering.scm (freecad): Update package > [inputs]: Move python-pyside-2-tools to native-inputs > [inputs]: Add pivy > [inputs]: Add qtxmlpatterns > [inputs]: Add qtwebkit Are all of these changes necessary to fix the errors? If not, we prefer to split the changes up into separate commits. For example, one commit to update the package, one commit to fix the errors, one commit to enable some optional feature (e.g. requiring qtwebkit). If all the changes must be made together, that's fine too. The commit message should be rewritten, but exactly how depends on answers to my previous questions. It could be written like this: ------ gnu: FreeCad: Update to 0.18.5-1.7616153. Fixes *description of bug*. * gnu/packages/engineering.scm (freecad): Update to 0.18.5-1.7616153. [inputs]: Add python-pivy, qtxmlpatterns, and qtwebkit. Remove python-pyside-2-tools. [native-inputs]: Add python-pyside-2-tools. ------ Can you send a revised patch series?
Hi, ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Thursday, December 31, 2020 11:36 PM, Leo Famulari <leo@famulari.name> wrote: > On Thu, Dec 31, 2020 at 06:47:59PM +0000, Ekaitz Zarraga wrote: > > > I attach 4 patches that correct the runtime issues freecad has and also supply some packages needed as a dependency. > > Freecad is a very complex package that is hard to build because many of its dependencies' buildsystem is broken. The approach followed here is the same that Nix follows and appears to work correctly. > > The changes correct the Draft module, which wasn't available because pivy was not added as a dependency. > > For context, see this message: > > https://lists.gnu.org/archive/html/guix-devel/2020-12/msg00123.html > > Thanks! > > Here is my feedback: > > > Subject: [PATCH 1/4] gnu: Add coin3D-4. > > > > * gnu/packages/graphics.scm (coin3D-4): New variable. > > > > > - (snippet > > > > > > - '(begin > > > > > > - (for-each delete-file > > > > > > - '("cfg/csubst.exe" > > > > > > - "cfg/wrapmsvc.exe")) > > > > > > Please add a brief code comment like "Delete binaries". > > > - (substitute* "CMakeLists.txt" > > > > > > - ((".*cpack.d.*") "")) > > > > > > - #t)))) > > > > > > What does this do? Please add an explanatory comment. It removes an unnecessary library. I'll add a comment. > > Subject: [PATCH 3/4] gnu: Add python-pivy. > > > > * gnu/packages/python-xyz.scm (python-pivy): New variable. > > > > > - (snippet > > > > > > - '(begin > > > > > > - (substitute* "CMakeLists.txt" > > > > > > - (("\\\\$\\\\{SoQt_INCLUDE_DIRS}") > > > > > > - "${Coin_INCLUDE_DIR};${SoQt_INCLUDE_DIRS}")) > > > > > > - #t)))) > > > > > > Origin snippets affect what is returned by`guix build --source python-pivy`, and are used for correcting very serious bugs in the > source code or for fixing so-called "freedom issues", such as removing > pre-compiled binaries. > > We prefer to make other types of changes in custom build phases in a > package's arguments. I'm not sure exactly what this substitution does — > please add a comment — but perhaps it would be more appropriate in a > custom build phase? This substitution corrects the repository. By default the repository doesn't find Coin3D, so we need to insert its search path by hand. What's the best way to add this? > > - (arguments > > - `(#:tests? #f)) > > > > > > Why are the tests disabled? We aim to make Guix packages pass upstream > test suites, so there should be a reason for skipping them. If there is > no test suite, just add a comment saying so. Same question about the > soqt package. > > > Subject: [PATCH 4/4] gnu: freecad correct runtime errors > > > > * gnu/packages/engineering.scm (freecad): Update package > > [inputs]: Move python-pyside-2-tools to native-inputs > > [inputs]: Add pivy > > [inputs]: Add qtxmlpatterns > > [inputs]: Add qtwebkit > > > > Are all of these changes necessary to fix the errors? If not, we prefer > to split the changes up into separate commits. For example, one commit > to update the package, one commit to fix the errors, one commit to > enable some optional feature (e.g. requiring qtwebkit). Ok, I'll separate the qtwebkit as it's not necessary for this fix. > If all the changes must be made together, that's fine too. > > The commit message should be rewritten, but exactly how depends on > answers to my previous questions. It could be written like this: > > ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- > > gnu: FreeCad: Update to 0.18.5-1.7616153. > > Fixes description of bug. > > - gnu/packages/engineering.scm (freecad): Update to 0.18.5-1.7616153. > [inputs]: Add python-pivy, qtxmlpatterns, and qtwebkit. Remove > python-pyside-2-tools. > [native-inputs]: Add python-pyside-2-tools. I'll change that. > Can you send a revised patch series? Of course. I'll write a follow up soon. Happy new year. Ekaitz
From 3c3fde8f5aa3433de076d6bbb90f9e60333f31b0 Mon Sep 17 00:00:00 2001 From: Ekaitz Zarraga <ekaitz@elenq.tech> Date: Thu, 31 Dec 2020 19:34:45 +0100 Subject: [PATCH 2/4] gnu: Add soqt. * gnu/packages/qt.scm (soqt): New variable. --- gnu/packages/qt.scm | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/gnu/packages/qt.scm b/gnu/packages/qt.scm index 2e621e758c..2af7e40734 100644 --- a/gnu/packages/qt.scm +++ b/gnu/packages/qt.scm @@ -67,6 +67,7 @@ #:use-module (gnu packages glib) #:use-module (gnu packages gnupg) #:use-module (gnu packages gperf) + #:use-module (gnu packages graphics) #:use-module (gnu packages gstreamer) #:use-module (gnu packages gtk) #:use-module (gnu packages icu4c) @@ -2824,3 +2825,36 @@ being fully customizable and easy to extend.") ;; According to LICENSE, either version 2 or version 3 of the GNU GPL may ;; be used. (license (list license:gpl2 license:gpl3)))) + + +(define-public soqt + (let ((commit-ref "fb8f655632bb9c9c60e0ff9fa69a5ba22d3ff99d") + (revision "1")) + (package + (name "soqt") + (version (git-version "1.6.0" revision commit-ref)) + (source + (origin + (method git-fetch) + (uri (git-reference + (url "https://github.com/coin3d/soqt") + (commit commit-ref) + (recursive? #t))) + (file-name (git-file-name name version)) + (sha256 + (base32 "16vikb3fy8rmk10sg5g0gy2c343hi3x7zccsga90ssnkzpq6m032")))) + (build-system cmake-build-system) + (arguments + '(#:tests? #f)) + (native-inputs + `(("pkg-config" ,pkg-config) + ("cmake" ,cmake))) + (inputs + `(("qtbase" ,qtbase) + ("coin3D-4" ,coin3D-4))) + (home-page "https://github.com/coin3d/soqt") + (synopsis "Qt GUI component toolkit library for Coin") + (description "SoQt is a Qt GUI component toolkit library for Coin. It is +also compatible with SGI and TGS Open Inventor, and the API is based on the API +of the InventorXt GUI component toolkit.") + (license license:bsd-3)))) -- 2.29.2