Package: guix-patches;
Reported by: Cayetano Santos <csantosb <at> inventati.org>
Date: Sat, 3 May 2025 17:52:02 UTC
Severity: normal
Tags: patch
Done: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Bug is archived. No further changes may be made.
Message #17 received at 78233 <at> debbugs.gnu.org (full text, mbox):
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> To: Cayetano Santos <csantosb <at> inventati.org> Cc: 78233 <at> debbugs.gnu.org, Gabriel Wicki <gabriel <at> erlikon.ch>, Ekaitz Zarraga <ekaitz <at> elenq.tech> Subject: Re: [bug#78233] [PATCH 1/2] gnu: Add nextpnr. Date: Thu, 08 May 2025 21:43:13 +0900
Hi!
Cayetano Santos <csantosb <at> inventati.org> writes:
> * gnu/packages/fpga.scm (nextpnr): New variable.
>
> Change-Id: Ic3476a6a4220ec20191897a6efb3d4aa347b51c2
> ---
> gnu/packages/fpga.scm | 78 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 78 insertions(+)
>
> diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
> index 2298dde595..9dbd1a4564 100644
> --- a/gnu/packages/fpga.scm
> +++ b/gnu/packages/fpga.scm
> @@ -178,6 +178,84 @@ (define-public iverilog
> ;; You have to accept both GPL2 and LGPL2.1+.
> (license (list license:gpl2 license:lgpl2.1+))))
>
> +(define nextpnr
> + (package
> + (name "nextpnr")
> + (version "0.8")
> + (source
> + (origin
> + (method git-fetch)
> + (uri (git-reference
> + (url "https://github.com/YosysHQ/nextpnr/")
> + (commit (string-append "nextpnr-" version))
> + ;; Required to get oourafft, json11, python-console and
> + ;; QtPropertyBrowser, not packaged in Guix.
> + (recursive? #t)))
I'd add a ';; TODO: Package the missing oourafft, json11, etc.'
dependencies.', so that people looking for things to do in the source
can find them.
> + (file-name (git-file-name name version))
> + (modules '((guix build utils)))
> + (snippet
> + ;; Remove bundled source code for which Guix has packages.
> + '(with-directory-excursion "3rdparty"
> + (for-each delete-file-recursively
> + '("googletest" "imgui" "pybind11" "qtimgui"
> + "sanitizers-cmake" "corrosion"))))
Great. Perhaps not necessary here, but I've used defensive techniques
in the past to allow newer release to slip new dependencies unknown to
the packager, e.g. in retroarch-minimal:
--8<---------------cut here---------------start------------->8---
(snippet
#~(begin
(use-modules (guix build utils)
(ice-9 ftw)
(srfi srfi-26))
;; XXX: 'delete-all-but' is copied from the turbovnc package.
(define (delete-all-but directory . preserve)
(define (directory? x)
(and=> (stat x #f)
(compose (cut eq? 'directory <>) stat:type)))
(with-directory-excursion directory
(let* ((pred
(negate (cut member <> (append '("." "..") preserve))))
(items (scandir "." pred)))
(for-each (lambda (item)
(if (directory? item)
(delete-file-recursively item)
(delete-file item)))
items))))
;; Remove as much bundled sources as possible, shaving off about
;; 65 MiB.
(delete-all-but "deps"
"feralgamemode" ;used in platform_unix.c
"mbedtls" ;further refined below
"yxml") ;used in rxml.c
;; This is an old root certificate used in net_socket_ssl_mbed.c,
;; not actually from mbedtls.
(delete-all-but "deps/mbedtls" "cacert.h")))
--8<---------------cut here---------------end--------------->8---
> + (sha256
> + (base32 "0p53a2gl89hf3hfwdxs6pykxyrk82j4lqpwd1fqia2y0c9r2gjlm"))))
> + (build-system qt-build-system)
> + (arguments
> + (list
> + #:cmake cmake ;CMake 3.25 or higher is required
There's also cmake-next, which would be easier to grep, perhaps, when
the time comes to sed the repo after a core upgrade of cmake-minimal.
> + #:configure-flags
> + #~(list "-DBUILD_GUI=OFF"
Why do we not build the GUI? We already have the Qt dependencies, it
seems.
> + "-DUSE_OPENMP=yes"
> + "-DBUILD_TESTS=ON"
> + (string-append "-DCURRENT_GIT_VERSION=nextpnr-" #$version)
> + "-DUSE_IPO=OFF")
> + #:phases
> + #~(modify-phases %standard-phases
> + ;; Remove references to unbundled code and link against external
> + ;; libraries instead.
> + (add-after 'unpack 'patch-source
> + (lambda* (#:key inputs #:allow-other-keys)
> + (substitute* "CMakeLists.txt"
> + ;; Use the system sanitizers-cmake module.
> + (("\\$\\{CMAKE_SOURCE_DIR\\}/3rdparty/sanitizers-cmake/cmake")
> + (string-append #$(this-package-native-input "sanitizers-cmake")
> + "/share/sanitizers-cmake/cmake"))
> + ;; Use the system googletest module
> + (("^\\s+add_subdirectory\\(3rdparty/googletest.*")
> + "")
> + ;; Use the system corrosion module
> + (("^\\s+add_subdirectory\\(3rdparty/corrosion.*")
> + "")
> + ;; replace gtest_main by gtest
> + (("^(\\s+target_link_libraries.*)( gtest_main)" _ prefix suffix)
> + (string-append prefix " gtest")))
> + ;; Use the system imgui module
> + (substitute* "gui/CMakeLists.txt"
> + (("\\$\\{CMAKE_SOURCE_DIR\\}(/3rdparty/imgui)")
> + (string-append #$(this-package-input "imgui")
> + "/include/imgui"))
> + (("\\$\\{CMAKE_SOURCE_DIR\\}(/3rdparty/qtimgui)")
> + (string-append #$(this-package-input "qtimgui")
> + "/include/qtimgui"))
> + (("^\\s+../3rdparty/(qt)?imgui.*")
> + "")))))))
Well done! If the source changes often, a patch could be more
maintainable; ideally in way that can be forwarded upstream too, with
good chances of being merged. That's less work for us in the long term.
--
Thanks,
Maxim
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.