Subject: Re: [bug#57774] [PATCH] Add qucs-s.

From: "(" <[email-masked]>
Date: Tue, 13 Sep 2022 17:12:36 +0100

Hey,

On Tue Sep 13, 2022 at 4:48 PM BST, Garek Dyszel via Guix-patches via wrote:
> +    (outputs '("out"))

This line is redundant; ``out'' is implicit.

> +    (propagated-inputs (list ngspice))

Can you try to avoid propagating this somehow? Propagation is a last-resort
measure, and usually we try to do something like patching invocations of
external commands to refer to the full store path. (Although, looking at
the ``modify-phases'' below, it seems like you already do that, so I guess
you'll only need to move ``ngspice'' to ``inputs''.)

> +    (arguments
> +     `(;; There exists no "tests" target in the Makefile generated by

Please use the new style ``(list ...)'' instead of ```(...)''.

> +       (modify-phases %standard-phases

You'll need to gexp this under the new arguments style: ``#~(modify-phases ...)''.

> +             (with-directory-excursion "etc/xdg/qucs_s"
> +               (invoke "touch" "qucs_s.conf")
> +               ;; Specify where the ngspice executable is located.
> +               (invoke "sed" "-i"
> +                       (string-append "1iNgspiceExecutable=/gnu/store/"
> +                                      (assoc-ref inputs "ngspice")
> +                                      "/bin/ngspice") "qucs_s.conf")
> +               ;; Set the spice4qucs working directory to
> +               ;; ~/.qucs/spice4qucs.
> +               ;; (The default is /spice4qucs, which is rather
> +               ;; dangerous.)
> +               (invoke "sed" "-i" "2iS4Q_workdir=~/.qucs/spice4qucs"
> +                       "qucs_s.conf")))))))

This looks pretty redundant. Try using this instead:

> +               (call-with-output-file "etc/xdg/qucs_s/qucs_s.conf"
> +                 (cute format <>
> +                       "~
> +NgspiceExecutable=~a
> +S4Q_workdir=~/.qucs/spice4qucs~%"
> +                       (search-input-file inputs "bin/ngspice")))

And add this to ``arguments'':

  #:modules `((ice-9 format)
              (srfi srfi-26)
              ,@%gnu-build-system-modules)

to import ``format'' and ``cute'', respectively.

> +    (description
> +     "Qucs-S is a spin-off of the Qucs cross-platform circuit
> +simulator.  The \"S\" letter indicates SPICE.  The purpose of the Qucs-S
> +subproject is to use free SPICE circuit simulation kernels with the
> +Qucs GUI.  It merges the power of SPICE and the simplicity of the Qucs
> +GUI.  Qucs intentionally uses its own SPICE incompatible simulation
> +kernel Qucsator.  It has advanced RF and AC domain simulation features,
> +but most of the existing industrial SPICE models are incompatible with
> +it.  Qucs-S is not a simulator by itself, but it requires to use a
> +simulation backend with it.  The schematic document format of Qucs and
> +Qucs-S are fully compatible.  Qucs-S can be used with the following
> +simulation kernels:
> +@itemize
> +@item Ngspice is recommended to use.  Ngspice is powerful
> +mixed-level/mixed-signal circuit simulator.  The most of industrial
> +SPICE models are compatible with Ngspice.  It has an excellent
> +performance for time-domain simulation of switching circuits and
> +powerful postprocessor.
> +@item XYCE is a new SPICE-compatible circuit simulator written by
> +Sandia from the scratch.  It supports basic SPICE simulation types and
> +has an advanced RF simulation features such as harmonic balance
> +simulation.
> +@item SpiceOpus is developed by the Faculty of Electrical Engineering
> +of the Ljubljana University.  It is based on the SPICE-3f5 code.
> +@item Backward compatible with Qucsator.
> +@end itemize")

I think this description would probably be better and far more succint as:

> +    (description
> +     "This package provides a fork of the Qucs cross-platform circuit
> +simulator, which allows the use of SPICE circuit simulation kernels
> +with the Qucs GUI.")

    -- (