[PATCH v2 0/6] Add FPGA Tools

[PATCH v2 0/6] Add FPGA Tools

[Danny Milosavljevic]

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

This patch adds FPGA tools.

Danny Milosavljevic (6):
  gnu: Add fpga module.
  gnu: fpga: Add abc.
  gnu: fpga: Add iverilog.
  gnu: fpga: Add yosys.
  gnu: fpga: Add icestorm.
  gnu: fpga: Add arachne-pnr.

 gnu/local.mk          |   1 +
 gnu/packages/fpga.scm | 271 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 272 insertions(+)
 create mode 100644 gnu/packages/fpga.scm

[PATCH v2 1/6] gnu: Add fpga module.

[Danny Milosavljevic]

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


* gnu/packages/fpga.scm: New file.
---
 gnu/local.mk          |  1 +
 gnu/packages/fpga.scm | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)
 create mode 100644 gnu/packages/fpga.scm


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-Add-fpga-module.patch --]
[-- Type: text/x-patch; name="0001-gnu-Add-fpga-module.patch", Size: 2233 bytes --]

diff --git a/gnu/local.mk b/gnu/local.mk
index 7416850..e70ba77 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -124,6 +124,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/packages/fltk.scm				\
   %D%/packages/fonts.scm			\
   %D%/packages/fontutils.scm			\
+  %D%/packages/fpga.scm				\
   %D%/packages/freedesktop.scm			\
   %D%/packages/freeipmi.scm			\
   %D%/packages/ftp.scm				\
diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
new file mode 100644
index 0000000..112d53b
--- /dev/null
+++ b/gnu/packages/fpga.scm
@@ -0,0 +1,40 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2016 Danny Milosavljevic <dannym@scratchpost.org>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu packages fpga)
+  #:use-module ((guix licenses) #:prefix license:)
+  #:use-module (guix packages)
+  #:use-module (guix download)
+  #:use-module (guix git-download)
+  #:use-module (guix build-system gnu)
+  #:use-module (guix build-system cmake)
+  #:use-module (gnu packages)
+  #:use-module (gnu packages pkg-config)
+  #:use-module (gnu packages tcl)
+  #:use-module (gnu packages readline)
+  #:use-module (gnu packages python)
+  #:use-module (gnu packages bison)
+  #:use-module (gnu packages flex)
+  #:use-module (gnu packages libffi)
+  #:use-module (gnu packages zip)
+  #:use-module (gnu packages perl)
+  #:use-module (gnu packages ghostscript)
+  #:use-module (gnu packages gawk)
+  #:use-module (gnu packages version-control)
+  #:use-module (gnu packages libftdi))
+

[PATCH v2 2/6] gnu: fpga: Add abc.

[Danny Milosavljevic]

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


* gnu/packages/fpga.scm (abc): New variable.
---
 gnu/packages/fpga.scm | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-gnu-fpga-Add-abc.patch --]
[-- Type: text/x-patch; name="0002-gnu-fpga-Add-abc.patch", Size: 2130 bytes --]

diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
index 112d53b..7571f87 100644
--- a/gnu/packages/fpga.scm
+++ b/gnu/packages/fpga.scm
@@ -38,3 +38,46 @@
   #:use-module (gnu packages version-control)
   #:use-module (gnu packages libftdi))
 
+;; To compile as C code (default):
+;;   make sure that CC=gcc and ABC_NAMESPACE is not defined.
+;; To compile as C++ code with namespaces:
+;;   make sure that CC=g++ and ABC_NAMESPACE is set to the namespace.
+;;   For example, add -DABC_NAMESPACE=xxx to OPTFLAGS.
+(define-public abc
+ (let ((commit "5ae4b975c49c"))
+  (package
+    (name "abc")
+    (version (string-append "0.0-" (string-take commit 7)))
+    (source (origin
+              (method url-fetch)
+              (uri
+               (string-append "https://bitbucket.org/alanmi/abc/get/"
+                              commit ".zip"))
+              (file-name (string-append name "-" version "-checkout.zip"))
+              (sha256
+                (base32
+                   "1syygi1x40rdryih3galr4q8yg1w5bvdzl75hd27v1xq0l5bz3d0"))))
+    (build-system gnu-build-system)
+    (native-inputs
+     `(("unzip" ,unzip)))
+    (inputs
+     `(("readline" ,readline)))
+    (arguments
+     `(#:tests? #f ; 'check target does not exist.
+       #:phases
+        (modify-phases %standard-phases
+          (delete 'configure)
+          (replace 'install
+            (lambda* (#:key outputs #:allow-other-keys)
+              (let* ((out (assoc-ref outputs "out"))
+                     (outbin (string-append out "/bin"))
+                     (target (string-append outbin "/abc")))
+                    (mkdir-p outbin)
+                    (copy-file "abc" target)))))))
+    (home-page "http://people.eecs.berkeley.edu/~alanmi/abc/")
+    (synopsis "Sequential Logic Synthesis and Formal Verification")
+    (description "ABC is a program for sequential logic synthesis
+and formal verification.")
+    ;; FIXME use license: MIT Modern variant as described in:
+    ;;   <https://fedoraproject.org/wiki/Licensing:MIT>
+    (license license:expat))))

[PATCH v2 3/6] gnu: fpga: Add iverilog.

[Danny Milosavljevic]

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


* gnu/packages/fpga.scm (iverilog): New variable.
---
 gnu/packages/fpga.scm | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0003-gnu-fpga-Add-iverilog.patch --]
[-- Type: text/x-patch; name="0003-gnu-fpga-Add-iverilog.patch", Size: 1474 bytes --]

diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
index 7571f87..b0d4819 100644
--- a/gnu/packages/fpga.scm
+++ b/gnu/packages/fpga.scm
@@ -81,3 +81,30 @@ and formal verification.")
     ;; FIXME use license: MIT Modern variant as described in:
     ;;   <https://fedoraproject.org/wiki/Licensing:MIT>
     (license license:expat))))
+
+(define-public iverilog
+  (package
+    (name "iverilog")
+    (version "10.1.1")
+    (source (origin
+              (method url-fetch)
+              (uri
+               (string-append "ftp://ftp.icarus.com/pub/eda/verilog/v10/verilog-" version ".tar.gz"))
+              (sha256
+                (base32
+                   "1nnassxvq30rnn0r2p85rkb2zwxk97p109y13x3vr365wzgpbapx"))))
+    (build-system gnu-build-system)
+    (native-inputs
+     `(("flex" ,flex)
+       ("bison" ,bison)
+       ("ghostscript" ,ghostscript) #| ps2pdf |#))
+    (home-page "http://iverilog.icarus.com/")
+    (synopsis "FPGA Verilog Simulation and Synthesis Tool")
+    (description "Icarus Verilog is a Verilog simulation and synthesis tool.
+It operates as a compiler, compiling source code written in Verilog
+(IEEE-1364) into some target format.
+For batch simulation, the compiler can generate an intermediate form called
+vvp assembly.
+This intermediate form is executed by the ``vvp'' command.
+For synthesis, the compiler generates netlists in the desired format.")
+    (license license:gpl2)))

[PATCH v2 4/6] gnu: fpga: Add yosys.

[Danny Milosavljevic]

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


* gnu/packages/fpga.scm (yosys): New variable.
---
 gnu/packages/fpga.scm | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0004-gnu-fpga-Add-yosys.patch --]
[-- Type: text/x-patch; name="0004-gnu-fpga-Add-yosys.patch", Size: 4416 bytes --]

diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
index b0d4819..0e51626 100644
--- a/gnu/packages/fpga.scm
+++ b/gnu/packages/fpga.scm
@@ -108,3 +108,84 @@ vvp assembly.
 This intermediate form is executed by the ``vvp'' command.
 For synthesis, the compiler generates netlists in the desired format.")
     (license license:gpl2)))
+
+(define-public yosys
+  (package
+    (name "yosys")
+    (version "0.6")
+    (source (origin
+              (method url-fetch)
+              (uri
+               (string-append "https://github.com/cliffordwolf/yosys/archive/"
+                              name "-" version ".tar.gz"))
+              (sha256
+                (base32
+                   "02j0c0m9dfyjccynalf0aggj6gy20k7iphpkg5cn6sdirlkv8gmx"))
+              (file-name (string-append name "-" version "-checkout.tar.gz"))
+              (modules '((guix build utils)))
+              (snippet
+                '(substitute* "Makefile"
+                   (("ABCREV = .*") "ABCREV = default\n")))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:test-target "test"
+       #:make-flags (list "CC=gcc"
+                          "CXX=g++"
+                          (string-append "PREFIX=" %output))
+       #:phases
+       (modify-phases %standard-phases
+         (replace 'configure
+           (lambda* (#:key inputs (make-flags '()) #:allow-other-keys)
+             (zero? (apply system* "make" "config-gcc" make-flags))))
+         (add-after 'configure 'prepare-abc
+           (lambda* (#:key inputs #:allow-other-keys)
+             (let* ((sourceabc (assoc-ref inputs "abc"))
+                    (sourcebin (string-append sourceabc "/bin"))
+                    (source (string-append sourcebin "/abc")))
+                   (mkdir-p "abc")
+                   (call-with-output-file "abc/Makefile"
+                     (lambda (port)
+                       (format port ".PHONY: all\nall:\n\tcp -f abc abc-default\n")))
+                   (copy-file source "abc/abc")
+                   (zero? (system* "chmod" "+w" "abc/abc")))))
+          (add-before 'check 'fix-iverilog-references
+             (lambda* (#:key inputs native-inputs #:allow-other-keys)
+               (let* ((xinputs (or native-inputs inputs))
+                      (xdirname (assoc-ref xinputs "iverilog"))
+                      (iverilog (string-append xdirname "/bin/iverilog")))
+                     (substitute* '("./manual/CHAPTER_StateOfTheArt/synth.sh"
+                                    "./manual/CHAPTER_StateOfTheArt/validate_tb.sh"
+                                    "./techlibs/ice40/tests/test_bram.sh"
+                                    "./techlibs/ice40/tests/test_ffs.sh"
+                                    "./techlibs/xilinx/tests/bram1.sh"
+                                    "./techlibs/xilinx/tests/bram2.sh"
+                                    "./tests/bram/run-single.sh"
+                                    "./tests/realmath/run-test.sh"
+                                    "./tests/simple/run-test.sh"
+                                    "./tests/techmap/mem_simple_4x1_runtest.sh"
+                                    "./tests/tools/autotest.sh"
+                                    "./tests/vloghtb/common.sh")
+                        (("if ! which iverilog") "if ! true")
+                        (("iverilog ") (string-append iverilog " "))
+                        (("iverilog_bin=\".*\"") (string-append "iverilog_bin=\""
+                                                                iverilog
+                                                                "\"")))))))))
+    ;; TODO add xdot [patch the path to it here] as soon as I find out where it is.
+    (native-inputs
+     `(("pkg-config" ,pkg-config)
+       ("python" ,python)
+       ("bison" ,bison)
+       ("flex" ,flex)
+       ("gawk" , gawk) ; for the tests and "make" progress pretty-printing
+       ("tcl" ,tcl) ; tclsh for the tests
+       ("iverilog" ,iverilog) ; for the tests
+    ))
+    (inputs
+     `(("tcl" ,tcl)
+       ("readline" ,readline)
+       ("libffi" ,libffi)
+       ("abc" ,abc)))
+    (home-page "http://www.clifford.at/yosys/")
+    (synopsis "FPGA Verilog RTL Synthesizer")
+    (description "Yosys synthesizes Verilog-2005.")
+    (license license:isc)))

[PATCH v2 5/6] gnu: fpga: Add icestorm.

[Danny Milosavljevic]

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


* gnu/packages/fpga.scm (icestorm): New variable.
---
 gnu/packages/fpga.scm | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0005-gnu-fpga-Add-icestorm.patch --]
[-- Type: text/x-patch; name="0005-gnu-fpga-Add-icestorm.patch", Size: 1931 bytes --]

diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
index 0e51626..25c4570 100644
--- a/gnu/packages/fpga.scm
+++ b/gnu/packages/fpga.scm
@@ -189,3 +189,40 @@ For synthesis, the compiler generates netlists in the desired format.")
     (synopsis "FPGA Verilog RTL Synthesizer")
     (description "Yosys synthesizes Verilog-2005.")
     (license license:isc)))
+
+(define-public icestorm
+  (let ((commit "12b2295c9087d94b75e374bb205ae4d76cf17e2f"))
+   (package
+    (name "icestorm")
+    (version (string-append "0.0-" (string-take commit 7)))
+    (source (origin
+              (method git-fetch)
+              (uri (git-reference
+                     (url "https://github.com/cliffordwolf/icestorm.git")
+                     (commit commit)))
+              (file-name (string-append name "-" version "-checkout"))
+              (sha256
+                (base32
+                   "1mmzlqvap6w8n4qzv3idvy51arkgn03692ssplwncy3akjrbsd2b"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:tests? #f ; no unit tests that don't need an FPGA exist.
+       #:make-flags (list "CC=gcc" "CXX=g++"
+                          (string-append "PREFIX=" (assoc-ref %outputs "out")))
+       #:phases
+        (modify-phases %standard-phases
+          (add-after 'unpack 'remove-usr-local
+            (lambda _
+              (substitute* "iceprog/Makefile"
+                (("-L/usr/local/lib") ""))))
+          (delete 'configure))))
+    (inputs
+      `(("libftdi" ,libftdi)))
+    (native-inputs
+      `(("python-3" ,python)
+        ("pkg-config" ,pkg-config)))
+    (home-page "http://www.clifford.at/icestorm/")
+    (synopsis "Project IceStorm - Lattice iCE40 FPGAs Bitstream Tools")
+    (description "Project IceStorm - Lattice iCE40 FPGAs Bitstream Tools. 
+Includes the actual FTDI connector.")
+    (license license:isc)))) ; FIXME more licenses?

[PATCH v2 6/6] gnu: fpga: Add arachne-pnr.

[Danny Milosavljevic]

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


* gnu/packages/fpga.scm (arachne-pnr): Add variable.
---
 gnu/packages/fpga.scm | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0006-gnu-fpga-Add-arachne-pnr.patch --]
[-- Type: text/x-patch; name="0006-gnu-fpga-Add-arachne-pnr.patch", Size: 2370 bytes --]

diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
index 25c4570..ed5bc18 100644
--- a/gnu/packages/fpga.scm
+++ b/gnu/packages/fpga.scm
@@ -226,3 +226,46 @@ For synthesis, the compiler generates netlists in the desired format.")
     (description "Project IceStorm - Lattice iCE40 FPGAs Bitstream Tools. 
 Includes the actual FTDI connector.")
     (license license:isc)))) ; FIXME more licenses?
+
+(define-public arachne-pnr
+  (let ((commit "52e69ed207342710080d85c7c639480e74a021d7"))
+   (package
+    (name "arachne-pnr")
+    (version (string-append "0.0-" (string-take commit 7)))
+    (source (origin
+              (method git-fetch)
+              (uri (git-reference
+                     (url "https://github.com/cseed/arachne-pnr.git")
+                     (commit commit)))
+              (file-name (string-append name "-" version "-checkout"))
+              (sha256
+                (base32
+                   "15bdw5yxj76lxrwksp6liwmr6l1x77isf4bs50ys9rsnmiwh8c3w"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:test-target "test"
+       #:phases (modify-phases %standard-phases
+       (replace 'configure
+         (lambda* (#:key outputs inputs #:allow-other-keys)
+           (substitute* '("Makefile")
+             (("DESTDIR = .*") (string-append "DESTDIR = "
+                                              (assoc-ref outputs "out")
+                                              "\n"))
+             (("ICEBOX = .*") (string-append "ICEBOX = "
+                                              (assoc-ref inputs "icestorm")
+                                              "/share/icebox\n")))
+           (substitute* '("./tests/fsm/generate.py"
+                          "./tests/combinatorial/generate.py")
+             (("#!/usr/bin/python") "#!/usr/bin/python2"))
+           #t)))))
+    (inputs
+     `(("icestorm" ,icestorm)))
+    (native-inputs
+     `(("git" ,git)  ; for determining its own version string
+       ("yosys" ,yosys) ; for tests
+       ("perl" ,perl) ; for shasum
+       ("python-2" ,python-2))) ; for tests
+    (home-page "https://github.com/cseed/arachne-pnr")
+    (synopsis "Place-and-Route Tool For FPGAs")
+    (description "Arachne-PNR is a Place-and-Route Tool For FPGAs.")
+    (license license:gpl2)))) ; FIXME check license

Re: [PATCH v2 3/6] gnu: fpga: Add iverilog.

[Thompson, David]

diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
index 7571f87..b0d4819 100644
--- a/gnu/packages/fpga.scm
+++ b/gnu/packages/fpga.scm
@@ -81,3 +81,30 @@ and formal verification.")
     ;; FIXME use license: MIT Modern variant as described in:
     ;;   <https://fedoraproject.org/wiki/Licensing:MIT>
     (license license:expat))))
+
+(define-public iverilog
+  (package
+    (name "iverilog")
+    (version "10.1.1")
+    (source (origin
+              (method url-fetch)
+              (uri
+               (string-append
"ftp://ftp.icarus.com/pub/eda/verilog/v10/verilog-" version
".tar.gz"))
+              (sha256
+                (base32
+                   "1nnassxvq30rnn0r2p85rkb2zwxk97p109y13x3vr365wzgpbapx"))))
+    (build-system gnu-build-system)
+    (native-inputs
+     `(("flex" ,flex)
+       ("bison" ,bison)
+       ("ghostscript" ,ghostscript) #| ps2pdf |#))

A minor nit, but we don't really use that form of comments, so I would
change it to this:

   ("ghostscript" ,ghostscript))) ; ps2pdf

+    (home-page "http://iverilog.icarus.com/")
+    (synopsis "FPGA Verilog Simulation and Synthesis Tool")
+    (description "Icarus Verilog is a Verilog simulation and synthesis tool.
+It operates as a compiler, compiling source code written in Verilog
+(IEEE-1364) into some target format.
+For batch simulation, the compiler can generate an intermediate form called
+vvp assembly.
+This intermediate form is executed by the ``vvp'' command.
+For synthesis, the compiler generates netlists in the desired format.")
+    (license license:gpl2)))

Is it really GPL2 only?

LGTM otherwise, thanks!

- Dave

Re: [PATCH v2 1/6] gnu: Add fpga module.

[Thompson, David]

On Tue, Aug 16, 2016 at 2:06 PM, Danny Milosavljevic
<dannym@scratchpost.org> wrote:
>
> * gnu/packages/fpga.scm: New file.
> ---
>  gnu/local.mk          |  1 +
>  gnu/packages/fpga.scm | 40 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
>  create mode 100644 gnu/packages/fpga.scm
>

You should merge this commit with the first patch that adds a variable
to this module.

- Dave

Re: [PATCH v2 2/6] gnu: fpga: Add abc.

[Thompson, David]

diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
index 112d53b..7571f87 100644
--- a/gnu/packages/fpga.scm
+++ b/gnu/packages/fpga.scm
@@ -38,3 +38,46 @@
   #:use-module (gnu packages version-control)
   #:use-module (gnu packages libftdi))

+;; To compile as C code (default):
+;;   make sure that CC=gcc and ABC_NAMESPACE is not defined.
+;; To compile as C++ code with namespaces:
+;;   make sure that CC=g++ and ABC_NAMESPACE is set to the namespace.
+;;   For example, add -DABC_NAMESPACE=xxx to OPTFLAGS.
+(define-public abc
+ (let ((commit "5ae4b975c49c"))
+  (package
+    (name "abc")
+    (version (string-append "0.0-" (string-take commit 7)))
+    (source (origin
+              (method url-fetch)
+              (uri
+               (string-append "https://bitbucket.org/alanmi/abc/get/"
+                              commit ".zip"))
+              (file-name (string-append name "-" version "-checkout.zip"))
+              (sha256
+                (base32
+                   "1syygi1x40rdryih3galr4q8yg1w5bvdzl75hd27v1xq0l5bz3d0"))))
+    (build-system gnu-build-system)
+    (native-inputs
+     `(("unzip" ,unzip)))
+    (inputs
+     `(("readline" ,readline)))
+    (arguments
+     `(#:tests? #f ; 'check target does not exist.
+       #:phases
+        (modify-phases %standard-phases

The opening paren should be aligned with the "#" in "#:phases" and the
rest of the form should be reindented accordingly.

+          (delete 'configure)
+          (replace 'install
+            (lambda* (#:key outputs #:allow-other-keys)
+              (let* ((out (assoc-ref outputs "out"))
+                     (outbin (string-append out "/bin"))
+                     (target (string-append outbin "/abc")))
+                (mkdir-p outbin)
+                (copy-file "abc" target)))))))

You over-indented by 4 spaces here and the line above it.

+    (home-page "http://people.eecs.berkeley.edu/~alanmi/abc/")
+    (synopsis "Sequential Logic Synthesis and Formal Verification")
+    (description "ABC is a program for sequential logic synthesis
+and formal verification.")
+    ;; FIXME use license: MIT Modern variant as described in:
+    ;;   <https://fedoraproject.org/wiki/Licensing:MIT>
+    (license license:expat))))

Could someone who knows more about this license chime in about what
the best thing to do is?

LGTM other than these few minor things.

- Dave

Re: [PATCH v2 2/6] gnu: fpga: Add abc.

[Leo Famulari]

On Tue, Aug 16, 2016 at 02:40:23PM -0400, Thompson, David wrote:
> +    ;; FIXME use license: MIT Modern variant as described in:
> +    ;;   <https://fedoraproject.org/wiki/Licensing:MIT>
> +    (license license:expat))))
> 
> Could someone who knows more about this license chime in about what
> the best thing to do is?

I would use the non-copyleft "license" variable.

Re: [PATCH v2 3/6] gnu: fpga: Add iverilog.

[Danny Milosavljevic]

On Tue, 16 Aug 2016 14:30:18 -0400
"Thompson, David" <dthompson2@worcester.edu> wrote:

> Is it really GPL2 only?

Most of the files are GPL2+, but these are GPL2 only:

- ./driver/iverilog.man.in
- ./iverilog-vpi.man.in
- ./tgt-fpga/iverilog-fpga.man
- ./vvp/vvp.man.in

In addition these files are LGPL2+:

./iverilog-vpi.sh
./tgt-vlog95/logic_lpm.c
./vpi/mt19937int.c
all the Makefile.in

And these are LGPL2.1+:

./tgt-vlog95/logic_lpm.c

Re: [PATCH v2 2/6] gnu: fpga: Add abc.

[Ricardo Wurmus]

Hi Danny,

thanks for the patch!

Although there are a couple of minor issues I think we can take it as is
and make a few changes before pushing to master.   Or you could send a
new version of the patch if you prefer that.

> * gnu/packages/fpga.scm (abc): New variable.
> ---
>  gnu/packages/fpga.scm | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)


> diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
> index 112d53b..7571f87 100644
> --- a/gnu/packages/fpga.scm
> +++ b/gnu/packages/fpga.scm
> @@ -38,3 +38,46 @@
>    #:use-module (gnu packages version-control)
>    #:use-module (gnu packages libftdi))
 
> +;; To compile as C code (default):
> +;;   make sure that CC=gcc and ABC_NAMESPACE is not defined.
> +;; To compile as C++ code with namespaces:
> +;;   make sure that CC=g++ and ABC_NAMESPACE is set to the namespace.
> +;;   For example, add -DABC_NAMESPACE=xxx to OPTFLAGS.

What does this mean for us?  Should we offer two packages?  Or is this
only needed for developer users of this package?  I’m inclined to just
drop this comment.  What do you think?

> +(define-public abc
> + (let ((commit "5ae4b975c49c"))

Others have already mentioned the cosmetic indentation issues, so I
won’t do it here :) Before applying the patch we just need to make sure
to run it through Emacs once more to be sure the indentation is
consistent.

We prefer to have the full commit here and abbreviate it in the version
string.  We usually also add a “revision” or “guix-revision” variable
(starting at 0 or 1).  That’s easier to update than having to modify the
version string directly each time the commit changes.

> +  (package
> +    (name "abc")
> +    (version (string-append "0.0-" (string-take commit 7)))

Here we normally take 9 characters of the hash.  The guix-internal
revision should be prefixed here.

> +    (source (origin
> +              (method url-fetch)
> +              (uri
> +               (string-append "https://bitbucket.org/alanmi/abc/get/"
> +                              commit ".zip"))
> +              (file-name (string-append name "-" version
> "-checkout.zip"))

I don’t think we need the “-checkout” part.

> +              (sha256
> +                (base32
> +                   "1syygi1x40rdryih3galr4q8yg1w5bvdzl75hd27v1xq0l5bz3d0"))))
> +    (build-system gnu-build-system)
> +    (native-inputs
> +     `(("unzip" ,unzip)))
> +    (inputs
> +     `(("readline" ,readline)))
> +    (arguments
> +     `(#:tests? #f ; 'check target does not exist.

“'check” (with the leading quote indicating that it is a Scheme symbol)
is a phase name in Guix.  It would be less confusing if the comment just
said

   “no check target”

because this is about a Makefile target, not about a Guix build phase.

> +       #:phases
> +        (modify-phases %standard-phases
> +          (delete 'configure)
> +          (replace 'install
> +            (lambda* (#:key outputs #:allow-other-keys)
> +              (let* ((out (assoc-ref outputs "out"))
> +                     (outbin (string-append out "/bin"))
> +                     (target (string-append outbin "/abc")))
> +                    (mkdir-p outbin)
> +                    (copy-file "abc" target)))))))

Please end the phase with #t as “copy-file” doesn’t have a specified
return value.  Instead of “copy-file” you could use this:

    (mkdir-p outbin)
    (install-file "abc" outbin)

“install-file” does not need to be given a target file name, just a
target directory.

> +    (home-page "http://people.eecs.berkeley.edu/~alanmi/abc/")
> +    (synopsis "Sequential Logic Synthesis and Formal Verification")

Let’s put this in lower case (except for the first word).

Thanks again!  Would you like me to take care of the changes or would
you prefer to send an updated patch?

~~ Ricardo

Re: [PATCH v2 2/6] gnu: fpga: Add abc.

[Danny Milosavljevic]

> What does this mean for us?  Should we offer two packages?  Or is this
> only needed for developer users of this package?  I’m inclined to just
> drop this comment.  What do you think?

I only install the binary and no library so it doesn't really matter. We can drop the comment - if we should need the library later we can find the comment in the mailing list archives.

> We prefer to have the full commit here and abbreviate it in the version
> string.  We usually also add a “revision” or “guix-revision” variable
> (starting at 0 or 1).  That’s easier to update than having to modify the
> version string directly each time the commit changes.

Yeah, back then I didn't take care of it. Sorry. Please add.

> “'check” (with the leading quote indicating that it is a Scheme symbol)
> is a phase name in Guix.  It would be less confusing if the comment just
> said
> 
>    “no check target”

Yeah.

> “install-file” does not need to be given a target file name, just a
> target directory.

Ok!

> > +    (home-page "http://people.eecs.berkeley.edu/~alanmi/abc/")
> > +    (synopsis "Sequential Logic Synthesis and Formal Verification")  
> 
> Let’s put this in lower case (except for the first word).
> 
> Thanks again!  Would you like me to take care of the changes or would
> you prefer to send an updated patch?

Yes please do.

Note that Leo also proposed to write

    (license (license:non-copyleft "https://fedoraproject.org/wiki/Licensing:MIT#Modern_Variants")))))

as license.

Re: [PATCH v2 2/6] gnu: fpga: Add abc.

[Danny Milosavljevic]

> > Thanks again!  Would you like me to take care of the changes or would
> > you prefer to send an updated patch?  
> 
> Yes please do.

Whoops. I just saw that I forgot to send yosys in the patch series. Therefore I'll send a new series with it and the fixes.

Re: [PATCH v2 2/6] gnu: fpga: Add abc.

[Ricardo Wurmus]

Danny Milosavljevic <dannym@scratchpost.org> writes:

>> > Thanks again!  Would you like me to take care of the changes or would
>> > you prefer to send an updated patch?  
>> 
>> Yes please do.
>
> Whoops. I just saw that I forgot to send yosys in the patch series. Therefore I'll send a new series with it and the fixes.

Oh, great.  Thank you very much for your efforts!
I hope this round of reviews wasn’t discouraging or frustrating.

~~ Ricardo

Re: [PATCH v2 2/6] gnu: fpga: Add abc.

[Pjotr Prins]

If you have time, also take a look at the Arrayfire, pocl and OpenCL stuff at

  https://github.com/genenetwork/guix-bioinformatics/tree/master/gn/packages

it is not finished, but a lot of work has gone in already.

Pj.

On Thu, Aug 18, 2016 at 05:10:12PM +0200, Danny Milosavljevic wrote:
> > > Thanks again!  Would you like me to take care of the changes or would
> > > you prefer to send an updated patch?  
> > 
> > Yes please do.
> 
> Whoops. I just saw that I forgot to send yosys in the patch series. Therefore I'll send a new series with it and the fixes.
> 

--