GNU bug report logs - #78063
[PATCH electronics-team] gnu: Add prjtrellis.

Previous Next

To reply to this bug, email your comments to 78063 AT debbugs.gnu.org.
There is no need to reopen the bug first.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Cayetano Santos <csantosb <at> inventati.org>
To: guix-patches <at> gnu.org
Cc: Cayetano Santos <csantosb <at> inventati.org>
Subject: [PATCH electronics-team] gnu: Add prjtrellis.
Date: Fri, 25 Apr 2025 20:23:15 +0200

* gnu/packages/electronics.scm (prjtrellis): New variable.

Change-Id: Iac188df00f55c06f9000fe1b688d6cded9d495fd
---
 gnu/packages/electronics.scm | 43 ++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/gnu/packages/electronics.scm b/gnu/packages/electronics.scm
index 12e44f234a..166de9e27c 100644
--- a/gnu/packages/electronics.scm
+++ b/gnu/packages/electronics.scm
@@ -415,6 +415,49 @@ (define-public openboardview
 @end itemize")
     (license license:expat)))
 
+(define-public prjtrellis
+  (package
+    (name "prjtrellis")
+    (version "1.4")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/YosysHQ/prjtrellis/")
+             (commit version)
+             (recursive? #t)))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "0c3asdfrjmnc6q3vawn3nfghgg43iajwy2zb8kck9d3wrypbhlmc"))))
+    (build-system cmake-build-system)
+    (arguments
+     (list
+      #:tests? #f ; tests are to be run from nextpnr-ecp5
+      #:phases
+      #~(modify-phases %standard-phases
+          (add-after 'unpack 'chdir
+            (lambda _
+              (chdir "libtrellis")))
+          ;; Remove bundled source code for which Guix has packages.
+          (add-after 'chdir 'remove-deps
+            (lambda _
+              (with-directory-excursion "3rdparty"
+                (for-each delete-file-recursively
+                          '("pybind11")))))
+          ;; point to pybind11 include dir
+          (add-after 'remove-deps 'setenv-pybind11
+            (lambda* (#:key inputs #:allow-other-keys)
+              (setenv "PYBIND11_INCLUDE_DIR"
+                      (string-append #$(this-package-input "pybind11")
+                                     "/include/pybind11")))))))
+    (native-inputs (list python))
+    (inputs (list openocd boost pybind11))
+    (synopsis "Placement and routing for ECP5 FPGAs")
+    (description
+     "Project Trellis is a Nextpnr backend compatible with ECP5 FPGAs.")
+    (home-page "https://github.com/YosysHQ/prjtrellis/")
+    (license license:expat)))
+
 (define-public pulseview
   (package
     (name "pulseview")

base-commit: 4fe4cf9fdd959126d3c53c3df4504d851e7b736a
-- 
2.49.0

Message #8 received at 78063 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Cayetano Santos <csantosb <at> inventati.org>
Cc: 78063 <at> debbugs.gnu.org, Ekaitz Zarraga <ekaitz <at> elenq.tech>
Subject: Re: [bug#78063] [PATCH electronics-team] gnu: Add prjtrellis.
Date: Fri, 02 May 2025 16:19:09 +0900

Hi,

Cayetano Santos <csantosb <at> inventati.org> writes:

> * gnu/packages/electronics.scm (prjtrellis): New variable.

[...]

> +(define-public prjtrellis
> +  (package
> +    (name "prjtrellis")
> +    (version "1.4")
> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://github.com/YosysHQ/prjtrellis/")
> +             (commit version)
> +             (recursive? #t)))

This needs an explanation, explaining why the bundled libraries/sources
in the checkout cannot be packaged separately: our standard practice is
to try to unbundle everything and build the components as distinct
packages.  Here it looks like it pulls some kind of database files used
by the project.  That's fine to use a recursive? #t for that, the reason
just needs to be explained.

[...]

> +      #:tests? #f ; tests are to be run from nextpnr-ecp5

nitpick: leave at least two spaces between the code and the ';' margin
comment, as ensured by Emacs with 'M-;'.  Also, I'm not sure I get what
it means: the test are run from another package?  Could you explain a
bit more in a standalone comment above the argument?

> +      #:phases
> +      #~(modify-phases %standard-phases
> +          (add-after 'unpack 'chdir
> +            (lambda _
> +              (chdir "libtrellis")))
> +          ;; Remove bundled source code for which Guix has packages.
> +          (add-after 'chdir 'remove-deps
> +            (lambda _
> +              (with-directory-excursion "3rdparty"
> +                (for-each delete-file-recursively
> +                          '("pybind11")))))

This would benefit from being done in the source directly, as a snippet
attached to the origin, to remove the extraneous files from the source
(which needlessly adds weigh to the source checkout).

But as I suggested above, perhaps recursive? #t can be removed, and the
few dependencies packaged separately, if they can be built as libraries.

> +          (add-after 'remove-deps 'setenv-pybind11
> +            (lambda* (#:key inputs #:allow-other-keys)
> +              (setenv "PYBIND11_INCLUDE_DIR"
> +                      (string-append #$(this-package-input "pybind11")
> +                                     "/include/pybind11")))))))
> +    (native-inputs (list python))
> +    (inputs (list openocd boost pybind11))
> +    (synopsis "Placement and routing for ECP5 FPGAs")
> +    (description
> +     "Project Trellis is a Nextpnr backend compatible with ECP5 FPGAs.")

The description is a bit too terse.  It'd be nice to know what features
are included the software provides, for example, or what are the various
commands are included (if there are many), and what they do.  You can
use a '@table @command' for this.

Otherwise it looks good to me.  Could you please submit a v2?

-- 
Thanks,
Maxim

Message #11 received at 78063 <at> debbugs.gnu.org (full text, mbox):

From: Cayetano Santos <csantosb <at> inventati.org>
To: 78063 <at> debbugs.gnu.org
Cc: Cayetano Santos <csantosb <at> inventati.org>
Subject: [PATCH electronics-team v2] gnu: Add prjtrellis.
Date: Fri,  2 May 2025 20:47:57 +0200

* gnu/packages/electronics.scm (prjtrellis): New variable.

Change-Id: Iac188df00f55c06f9000fe1b688d6cded9d495fd
---
 gnu/packages/electronics.scm | 59 ++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/gnu/packages/electronics.scm b/gnu/packages/electronics.scm
index d63511ff22..01a9f68f52 100644
--- a/gnu/packages/electronics.scm
+++ b/gnu/packages/electronics.scm
@@ -422,6 +422,65 @@ (define-public openboardview
 @end itemize")
     (license license:expat)))
 
+(define-public prjtrellis
+  (package
+    (name "prjtrellis")
+    (version "1.4")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/YosysHQ/prjtrellis/")
+             (commit version)
+             ;; pulls the bitstream db for ECP5 devices; this is useful only by
+             ;; prjtrellis, there is no need to package it separately
+             (recursive? #t)))
+       (file-name (git-file-name name version))
+       (modules '((guix build utils)))
+       (snippet
+        ;; Remove bundled source code for which Guix has packages.
+        '(with-directory-excursion "libtrellis/3rdparty"
+           (for-each delete-file-recursively
+                     '("pybind11"))))
+       (sha256
+        (base32 "0c3asdfrjmnc6q3vawn3nfghgg43iajwy2zb8kck9d3wrypbhlmc"))))
+    (build-system cmake-build-system)
+    (arguments
+     (list
+      ;; the examples test directory requires nextpnr, using this package as a
+      ;; backend, which is provided by nextpnr-ecp5; the tests are to be run in
+      ;; there then
+      #:tests? #f
+      #:phases
+      #~(modify-phases %standard-phases
+          (add-after 'unpack 'chdir
+            (lambda _
+              (chdir "libtrellis")))
+          ;; point to pybind11 include dir
+          (add-after 'chdir 'setenv-pybind11
+            (lambda* (#:key inputs #:allow-other-keys)
+              (setenv "PYBIND11_INCLUDE_DIR"
+                      (string-append #$(this-package-input "pybind11")
+                                     "/include/pybind11")))))))
+    (native-inputs (list python))
+    (inputs (list openocd boost pybind11))
+    (synopsis "Placement and routing for ECP5 FPGAs")
+    (description
+     "Project Trellis is a Nextpnr backend compatible with ECP5 FPGAs.
+The following features are currently available:
+@table @command
+@item logic slice functionality, including carries
+@item distributed RAM inside logic slices
+@item all internal interconnect
+@item basic IO, including tristate
+@item block RAM, using inference or manual instantiation
+@item multipliers using manual instantiation
+@item global networks and PLLs
+@item transcievers (DCUs)
+@end table")
+    (home-page "https://github.com/YosysHQ/prjtrellis/")
+    (license license:expat)))
+
 (define-public opensta
   ;; There are no releases, we use last commit.
   (let ((commit "eb8d39a7dd81b5ca2582ad9bbce0fb6e094b3e0f")

base-commit: 0b83a27b67ae92e988795322ae988518ec3e6972
--
2.49.0

Message #14 received at 78063 <at> debbugs.gnu.org (full text, mbox):

From: Cayetano Santos <csantosb <at> inventati.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 78063 <at> debbugs.gnu.org, Ekaitz Zarraga <ekaitz <at> elenq.tech>
Subject: Re: [bug#78063] [PATCH electronics-team] gnu: Add prjtrellis.
Date: Fri, 02 May 2025 21:04:45 +0200

[Message part 1 (text/plain, inline)]

>ven. 02 mai 2025 at 16:19, Maxim Cournoyer <maxim.cournoyer <at> gmail.com> wrote:

Hi Maxim,

>> +      #:tests? #f ; tests are to be run from nextpnr-ecp5
>
> nitpick: leave at least two spaces between the code and the ';' margin
> comment, as ensured by Emacs with 'M-;'.  Also, I'm not sure I get what
> it means: the test are run from another package?  Could you explain a
> bit more in a standalone comment above the argument?

The idea is the following, see #77114. We spin-off a common nextpnr
package, from which nextpnr-ice40 inherits, using icestorm as a backend.

We do the same with nextpnr-ecp5, which inherits from nextpnr, and uses
prjtrellis as a backend. Others will follow, see [0].

This makes that example tests in prjtrellis, are to be tested under
nextpnr-ecp5. Prjtrellis, by itself, cannot do much. Hope it is clear.

> Otherwise it looks good to me.  Could you please submit a v2?

Thanks for your review, I just sent v2, hopefully, including all your
improvements.

C.

[0] https://github.com/YosysHQ/nextpnr/blob/master/README.md

[signature.asc (application/pgp-signature, inline)]

Message #19 received at 78063-done <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Cayetano Santos <csantosb <at> inventati.org>
Cc: Ekaitz Zarraga <ekaitz <at> elenq.tech>, 78063-done <at> debbugs.gnu.org
Subject: Re: [bug#78063] [PATCH electronics-team] gnu: Add prjtrellis.
Date: Thu, 08 May 2025 14:49:09 +0900

Hi,

Cayetano Santos <csantosb <at> inventati.org> writes:

>>ven. 02 mai 2025 at 16:19, Maxim Cournoyer <maxim.cournoyer <at> gmail.com> wrote:
>
> Hi Maxim,
>
>>> +      #:tests? #f ; tests are to be run from nextpnr-ecp5
>>
>> nitpick: leave at least two spaces between the code and the ';' margin
>> comment, as ensured by Emacs with 'M-;'.  Also, I'm not sure I get what
>> it means: the test are run from another package?  Could you explain a
>> bit more in a standalone comment above the argument?
>
> The idea is the following, see #77114. We spin-off a common nextpnr
> package, from which nextpnr-ice40 inherits, using icestorm as a backend.
>
> We do the same with nextpnr-ecp5, which inherits from nextpnr, and uses
> prjtrellis as a backend. Others will follow, see [0].
>
> This makes that example tests in prjtrellis, are to be tested under
> nextpnr-ecp5. Prjtrellis, by itself, cannot do much. Hope it is clear.
>
>> Otherwise it looks good to me.  Could you please submit a v2?
>
> Thanks for your review, I just sent v2, hopefully, including all your
> improvements.

Looks good!  I've made the standalone comments punctuation, and turned
the table into an itemized list (a table has two columns):

--8<---------------cut here---------------start------------->8---
modified   gnu/packages/electronics.scm
@@ -432,8 +432,8 @@ (define-public prjtrellis
        (uri (git-reference
              (url "https://github.com/YosysHQ/prjtrellis/")
              (commit version)
-             ;; pulls the bitstream db for ECP5 devices; this is useful only by
-             ;; prjtrellis, there is no need to package it separately
+             ;; Pull the bitstream database for ECP5 devices; this is useful
+             ;; only by prjtrellis: there is no need to package it separately.
              (recursive? #t)))
        (file-name (git-file-name name version))
        (modules '((guix build utils)))
@@ -447,9 +447,9 @@ (define-public prjtrellis
     (build-system cmake-build-system)
     (arguments
      (list
-      ;; the examples test directory requires nextpnr, using this package as a
-      ;; backend, which is provided by nextpnr-ecp5; the tests are to be run in
-      ;; there then
+      ;; The examples test directory requires nextpnr, using this package as a
+      ;; backend, which is provided by nextpnr-ecp5: the tests are to be run
+      ;; in this later package.
       #:tests? #f
       #:phases
       #~(modify-phases %standard-phases
@@ -468,7 +468,7 @@ (define-public prjtrellis
     (description
      "Project Trellis is a Nextpnr backend compatible with ECP5 FPGAs.
 The following features are currently available:
-@table @command
+@itemize
 @item logic slice functionality, including carries
 @item distributed RAM inside logic slices
 @item all internal interconnect
@@ -476,8 +476,8 @@ (define-public prjtrellis
 @item block RAM, using inference or manual instantiation
 @item multipliers using manual instantiation
 @item global networks and PLLs
-@item transcievers (DCUs)
-@end table")
+@item transcievers (DCUs.)
+@end itemize")
     (home-page "https://github.com/Yos
--8<---------------cut here---------------end--------------->8---

Pushed as 2ca8d382816.

-- 
Thanks,
Maxim

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.