[bug#73918] [PATCH v1 1/2] gnu: iverilog: Fix yosys tests

[bug#73918] [PATCH v1 1/2] gnu: iverilog: Fix yosys tests

[Jakob Kirsch via Guix-patches via]

* gnu/packages/fpga.scm (iverilog): [inputs]: Add zlib.

Change-Id: I961bf73b17f96a5891232a8646bc04581dfb8bfc
---
 gnu/packages/fpga.scm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
index 545ec3482d..153d17d1f3 100644
--- a/gnu/packages/fpga.scm
+++ b/gnu/packages/fpga.scm
@@ -125,6 +125,7 @@ (define-public iverilog
       #:make-flags #~(list (string-append "PREFIX="
                                           #$output))
       #:bootstrap-scripts #~(list "autoconf.sh")))
+    (inputs (list zlib))
     (native-inputs (list autoconf bison flex gperf))
     (home-page "https://steveicarus.github.io/iverilog")
     (synopsis "FPGA Verilog simulation and synthesis tool")

base-commit: 5703914e93d81ac6037240582abe899282e78f15
--
2.46.0

[bug#73918] [PATCH v2 1/2] gnu: iverilog: Fix yosys tests

[Jakob Kirsch via Guix-patches via]

* gnu/packages/fpga.scm (iverilog): [inputs]: Add zlib.

Change-Id: I961bf73b17f96a5891232a8646bc04581dfb8bfc
---
 gnu/packages/fpga.scm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
index 545ec3482d..153d17d1f3 100644
--- a/gnu/packages/fpga.scm
+++ b/gnu/packages/fpga.scm
@@ -125,6 +125,7 @@ (define-public iverilog
       #:make-flags #~(list (string-append "PREFIX="
                                           #$output))
       #:bootstrap-scripts #~(list "autoconf.sh")))
+    (inputs (list zlib))
     (native-inputs (list autoconf bison flex gperf))
     (home-page "https://steveicarus.github.io/iverilog")
     (synopsis "FPGA Verilog simulation and synthesis tool")

base-commit: 5703914e93d81ac6037240582abe899282e78f15
--
2.46.0

[bug#73918] [PATCH v2 2/2] gnu: yosys: Update to 0.46.

[Jakob Kirsch via Guix-patches via]

* gnu/packages/fpga.scm (yosys): Update to 0.46.

Change-Id: Ibdd4c02155ace62bf50ff9b7bb605edc21698c2a
---
 gnu/packages/fpga.scm | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
index 153d17d1f3..83e81bfc50 100644
--- a/gnu/packages/fpga.scm
+++ b/gnu/packages/fpga.scm
@@ -8,6 +8,7 @@
 ;;; Copyright © 2022 Christian Gelinek <cgelinek@radlogic.com.au>
 ;;; Copyright © 2022 jgart <jgart@dismail.de>
 ;;; Copyright © 2024 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2024 Jakob Kirsch <jakob.kirsch@web.de>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -149,15 +150,15 @@ (define-public iverilog
 (define-public yosys
   (package
     (name "yosys")
-    (version "0.26")
+    (version "0.46")
     (source (origin
               (method git-fetch)
               (uri (git-reference
                     (url "https://github.com/YosysHQ/yosys")
-                    (commit (string-append "yosys-" version))))
+                    (commit version)))
               (sha256
                (base32
-                "0s79ljgbcfkm7l9km7dcvlz4mnx38nbyxppscvh5il5lw07n45gx"))
+                "1zj7vbpy6v1wn4p5cjs4hdjd467a1j1aj2qhs148bl2s6mzq3p86"))
               (file-name (git-file-name name version))))
     (build-system gnu-build-system)
     (arguments
--
2.46.0

[bug#73918] [PATCH] gnu: yosys: Update to 0.46.

[Denis 'GNUtoo' Carikli]

[-- Attachment #1: Type: text/plain, Size: 704 bytes --]

Hi,

Thanks a lot for the patches. 

Note that I'm not a Guix committer nor maintainer.

Also I'm not familiar with best practice for commit messages within the
Guix projects but other projects that care a lot about code quality
tend to indicate the commit that broke something in the commit message.

For instance in Linux we have commits like that:
> This was broken by commit 16aac5ad1fa9 ("ovl: support encoding
> non-decodable file handles"), which didn't take the lower-only
> configuration into account.

That would simplify the review as here a reviewer will probably need to
look into what broke yosys to verify if your fix is the best possible
fix (it probably is).

Denis.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#73918] [PATCH] gnu: yosys: Update to 0.46.

[Jakob Kirsch via Guix-patches via]

Thank you for the feedback, the commit that broke yosys was b32f8bc9da74158330cebdbcb481d41e1be6d1ce when iverilog was updated to 12.0

iverilog needs zlib as an input for a feature that yosys requires.

I fixed that with the one patch and also updated yosys in the process because it's kinda outdated.

[bug#73918] (no subject)

[Cayetano Santos via Guix-patches via]

As the author of the faulty commit, blindly removing zlib, I can
confirm: this patch fixes the test issues with yosys and builds yosys
0.46. Thanks !

[bug#73918] QA review for 73918

[Cayetano Santos via Guix-patches via]

user guix
usertag 73918 + waiting-on-contributor
thanks

Guix QA review form submission:

- There is a missing dot at the end of commit message in v2 1/2

- If you send a v3, it’s better to include a link to the commit
  introducing the error you’re fixing in the body of the commit message

    https://git.savannah.gnu.org/cgit/guix.git/commit/?id=b32f8bc9da

- [linter yosys] : "bash-minimal" should be in 'inputs' when
  'wrap-program' is used, you’ll need also

    #:use-module (gnu packages bash)

Other than this, it builds and executes with no issue

Thanks for fixing the error !

Items marked as checked: Commit messages, Package builds, Lint warnings

[bug#73918] [PATCH] gnu: yosys: Update to 0.46.

[Denis 'GNUtoo' Carikli]

[-- Attachment #1: Type: text/plain, Size: 1350 bytes --]

Hi,

> Thank you for the feedback, the commit that broke yosys was
> b32f8bc9da74158330cebdbcb481d41e1be6d1ce when iverilog was updated to
> 12.0
Thanks a lot for the information. I also have yosys that fails because
of the missing zlib but it wasn't clear why it became needed out of the
blue. I've not build-tested the patch yet though.

The commit b32f8bc9da74158330cebdbcb481d41e1be6d1ce ("gnu: iverilog:
Update to 12.0."), also brings more questions. In it we have:
-    (native-inputs
-     (list flex bison ghostscript zlib))   ; ps2pdf
+    (native-inputs (list autoconf bison flex gperf))

So do we also need Ghostscript ? Though if it builds and run fine
without Ghostscript maybe ghostscript got removed because it wasn't
useful?

Also would it help if I package some functional test(s) for the FPGA
toolchain? Personally I found that it made it easier to test the
packages when I maintained them for Parabola.

Though the "functional test" I have is just the FOMU workshop code that
I modified to blink blue instead of other colors:
https://git.parabola.nu/fpga/fomu-verilog-blue-blink.git/

The downside is that tests like that are specific to an FPGA model so
I'm unsure if this is a good idea or not.

Also note that I don't know FPGAs well. I just got one to maintain some
Parabola packages.

Denis.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#73918] [PATCH] gnu: yosys: Update to 0.46.

[Cayetano Santos via Guix-patches via]

> I've not build-tested the patch yet though.

I have, It works like a charm.

> So do we also need Ghostscript ? Though if it builds and run fine
> without Ghostscript maybe ghostscript got removed because it wasn't
> useful?

I got removed (by me) because there is no mention to in the instructions
for installing iverilog (see readme), same as zlib. It turns out that
the later is important, though.

To that purpose, the command (that I just discovered)

    guix refresh --list-dependent iverilog

outputs a list of packages (yosys here) which depends on iverilog, and
need to be rebuild upon every new modification in iverilog. If I had run
it, I would have discovered the issue on time. My bad.

In addition, having a look at the history of latest iverilog related
commits is not a bad idea.

    https://git.savannah.gnu.org/cgit/guix.git/commit/?id=9f82d1f1c0

C.

[bug#73918] [PATCH v3 0/2] Minor corrections to v2

[Cayetano Santos via Guix-patches via]

Add modifications neeeded as for previous QA review:

. missing dot at the end of commit message in v2 1/2
. include a link in the commit message to the commit introducing the error this patch fixes

    https://git.savannah.gnu.org/cgit/guix.git/commit/?id=b32f8bc9da

. "bash-minimal" should be in 'inputs' when 'wrap-program' is used

Cayetano Santos (2):
  gnu: iverilog: Fix yosys tests.
  gnu: yosys: Update to 0.46.

 gnu/packages/fpga.scm | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

--
2.46.0

[bug#73918] [PATCH v3 1/2] gnu: iverilog: Fix yosys tests.

[Cayetano Santos via Guix-patches via]

Change-Id: I262db5db43527a3a2a1753163f7b9a4104f7e895

Fixes issue introduced in

  https://git.savannah.gnu.org/cgit/guix.git/commit/?id=b32f8bc9da

which removes zlib. This produces errors when building yosys.

Change-Id: I5b817d176430a0e717597482883d7e806c4552ab
---

See explanation in commit message.

 gnu/packages/fpga.scm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
index c812ed3b7e..ea08aed04c 100644
--- a/gnu/packages/fpga.scm
+++ b/gnu/packages/fpga.scm
@@ -125,7 +125,7 @@ (define-public iverilog
       #:make-flags #~(list (string-append "PREFIX="
                                           #$output))
       #:bootstrap-scripts #~(list "autoconf.sh")))
+    (inputs (list zlib))
     (native-inputs (list autoconf bison flex gperf))
     (home-page "https://steveicarus.github.io/iverilog")
     (synopsis "FPGA Verilog simulation and synthesis tool")
--
2.46.0

[bug#73918] [PATCH v3 2/2] gnu: yosys: Update to 0.46.

[Cayetano Santos via Guix-patches via]

* gnu/packages/fpga.scm (yosys): Update to 0.46.

Change-Id: Id980679e6576b27429611f8399eff2185e57d684
---
 gnu/packages/fpga.scm | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
index ea08aed04c..be3e57ffb4 100644
--- a/gnu/packages/fpga.scm
+++ b/gnu/packages/fpga.scm
@@ -8,6 +8,7 @@
 ;;; Copyright © 2022 Christian Gelinek <cgelinek@radlogic.com.au>
 ;;; Copyright © 2022 jgart <jgart@dismail.de>
 ;;; Copyright © 2024 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2024 Jakob Kirsch <jakob.kirsch@web.de>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -38,6 +39,7 @@ (define-module (gnu packages fpga)
   #:use-module (gnu packages autotools)
   #:use-module (gnu packages algebra)
   #:use-module (gnu packages base)
+  #:use-module (gnu packages bash)
   #:use-module (gnu packages bison)
   #:use-module (gnu packages boost)
   #:use-module (gnu packages check)
@@ -149,15 +151,15 @@ (define-public iverilog
 (define-public yosys
   (package
     (name "yosys")
-    (version "0.26")
+    (version "0.46")
     (source (origin
               (method git-fetch)
               (uri (git-reference
                     (url "https://github.com/YosysHQ/yosys")
-                    (commit (string-append "yosys-" version))))
+                    (commit version)))
               (sha256
                (base32
-                "0s79ljgbcfkm7l9km7dcvlz4mnx38nbyxppscvh5il5lw07n45gx"))
+                "1zj7vbpy6v1wn4p5cjs4hdjd467a1j1aj2qhs148bl2s6mzq3p86"))
               (file-name (git-file-name name version))))
     (build-system gnu-build-system)
     (arguments
@@ -216,6 +218,7 @@ (define-public yosys
            tcl)) ; tclsh for the tests
     (inputs
      (list abc
+           bash-minimal
            graphviz
            gtkwave
            libffi
-- 
2.46.0

bug#73918: [PATCH v3 2/2] gnu: yosys: Update to 0.46.

[Maxim Cournoyer]

Hello,

Cayetano Santos <csantosb@inventati.org> writes:

> * gnu/packages/fpga.scm (yosys): Update to 0.46.
>
> Change-Id: Id980679e6576b27429611f8399eff2185e57d684

Merged this as 4bfe5fc584 and followed with an update to 0.47.

Closing!

-- 
Thanks,
Maxim

[bug#73918] QA review for 73918

[Cayetano Santos via Guix-patches via]

user guix
usertag 73918 + reviewed-looks-good
thanks

Guix QA review form submission:

Items marked as checked:

- Commit messages
- Package builds
- Lint warnings
- Package tests
- Package style
- List dependent packages build