[PATCH 0/5] gnu: yosys: Update to 0.24.

Here's a patch series that aims to improve the package for Yosys, a tool for

synthesizing Verilog RTL designs, by

- Updating the source-repository and home-page URLs to their current location;

- Updating the package definition to the modern style, using gexps and

search-input-file;

- Having Yosys use the "abc" executable from Guix's package rather than

creating and installing a duplicate copy;

- Explicitly propagating packages with executables Yosys invokes at runtime

rather than embedding store references in Yosys' code; and

- Updating Yosys to version 0.24.

I've tested building the package and its dependencies on x86-64 and AArch64

and everything seems okay[0]. Again, arachne-pnr fails to build in any case

and will need separate attention.

[0] Yosys' README.md file includes sample sessions useful for testing,

particularly whether external tools can be invoked via the "show" and

"abc" commands:

https://github.com/YosysHQ/yosys/blob/3ebc50d/README.md#getting-started

The files "fiedler-cooley.v" and "mycells.lib" are present in the source

tree.

--

Simon South

simon@simonsouth.net

Simon South (5):

gnu: yosys: Update source and home-page URLs.

gnu: yosys: Use new package style.

gnu: yosys: Use external abc.

gnu: yosys: Propagate external dependencies.

gnu: yosys: Update to 0.24.

gnu/packages/fpga.scm | 120 ++++++++++++++++--------------------------

1 file changed, 45 insertions(+), 75 deletions(-)

base-commit: 01762b7171f9afaf7ffa364f7926461bdebc903f

--

2.38.1

[home-page]: Update URL.

---

gnu/packages/fpga.scm | 4 ++--

1 file changed, 2 insertions(+), 2 deletions(-)

"modules" field.

[arguments]: Replace "prepare-abc" phase with "use-external-abc", which

configures the package's build system to use the system's "abc" executable

instead of creating a duplicate; add "add-symbolic-link" phase to preserve

availability of "yosys-abc" command.

[inputs]: Move abc from here...

[propagated-inputs]: ...to here, to ensure its availability at runtime.

---

gnu/packages/fpga.scm | 35 +++++++++++++++++------------------

1 file changed, 17 insertions(+), 18 deletions(-)

explicitly return #t.

[arguments]: Use gexps; use search-input-file to locate executables; simplify

parameters to "configure" phase; don't explicitly return #t from phases.

[native-inputs]: Sort alphabetically.

[inputs]: Sort alphabetically.

---

gnu/packages/fpga.scm | 122 +++++++++++++++++++++---------------------

1 file changed, 60 insertions(+), 62 deletions(-)

here...

[propagated-inputs]: ...to here, to ensure the availability at runtime of

executables invoked by yosys' "show" command.

[arguments]: Remove now-obsolete "fix-paths" phase.

---

gnu/packages/fpga.scm | 20 +++++---------------

1 file changed, 5 insertions(+), 15 deletions(-)

[arguments]: Remove obsolete "fix-iverilog-references" phase.

[inputs]: Add zlib.

[propagated-inputs]: Add gtkwave, python, python-click.

---

gnu/packages/fpga.scm | 35 +++++++++--------------------------

1 file changed, 9 insertions(+), 26 deletions(-)

Here's a revised patchset with a modified final patch that updates yosys to

version 0.25, released in the past week, instead of 0.24. Everything else

remains unchanged.

I've re-tested this on x86-64 and AArch64 and everything appears fine.

Here's the original cover letter:

Here's a patch series that aims to improve the package for Yosys, a tool for

synthesizing Verilog RTL designs, by

- Updating the source-repository and home-page URLs to their current location;

- Updating the package definition to the modern style, using gexps and

search-input-file;

- Having Yosys use the "abc" executable from Guix's package rather than

creating and installing a duplicate copy;

- Explicitly propagating packages with executables Yosys invokes at runtime

rather than embedding store references in Yosys' code; and

- Updating Yosys to version 0.24.

I've tested building the package and its dependencies on x86-64 and AArch64

and everything seems okay[0]. Again, arachne-pnr fails to build in any case

and will need separate attention.

[0] Yosys' README.md file includes sample sessions useful for testing,

particularly whether external tools can be invoked via the "show" and

"abc" commands:

https://github.com/YosysHQ/yosys/blob/3ebc50d/README.md#getting-started

The files "fiedler-cooley.v" and "mycells.lib" are present in the source

tree.

--

Simon South

simon@simonsouth.net

Simon South (5):

gnu: yosys: Update source and home-page URLs.

gnu: yosys: Use new package style.

gnu: yosys: Use external abc.

gnu: yosys: Propagate external dependencies.

gnu: yosys: Update to 0.25.

gnu/packages/fpga.scm | 120 ++++++++++++++++--------------------------

1 file changed, 45 insertions(+), 75 deletions(-)

base-commit: f5ea392b9cd3851ed7517876a8cd4beb13893c9c

--

2.38.1

"modules" field.

[arguments]: Replace "prepare-abc" phase with "use-external-abc", which

configures the package's build system to use the system's "abc" executable

instead of creating a duplicate; add "add-symbolic-link" phase to preserve

availability of "yosys-abc" command.

[inputs]: Move abc from here...

[propagated-inputs]: ...to here, to ensure its availability at runtime.

---

gnu/packages/fpga.scm | 35 +++++++++++++++++------------------

1 file changed, 17 insertions(+), 18 deletions(-)

[home-page]: Update URL.

---

gnu/packages/fpga.scm | 4 ++--

1 file changed, 2 insertions(+), 2 deletions(-)

here...

[propagated-inputs]: ...to here, to ensure the availability at runtime of

executables invoked by yosys' "show" command.

[arguments]: Remove now-obsolete "fix-paths" phase.

---

gnu/packages/fpga.scm | 20 +++++---------------

1 file changed, 5 insertions(+), 15 deletions(-)

explicitly return #t.

[arguments]: Use gexps; use search-input-file to locate executables; simplify

parameters to "configure" phase; don't explicitly return #t from phases.

[native-inputs]: Sort alphabetically.

[inputs]: Sort alphabetically.

---

gnu/packages/fpga.scm | 122 +++++++++++++++++++++---------------------

1 file changed, 60 insertions(+), 62 deletions(-)

[arguments]: Remove obsolete "fix-iverilog-references" phase.

[inputs]: Add zlib.

[propagated-inputs]: Add gtkwave, python, python-click.

---

gnu/packages/fpga.scm | 35 +++++++++--------------------------

1 file changed, 9 insertions(+), 26 deletions(-)

Simon South <simon@simonsouth.net> writes:

Thanks Simon, I've pushed the first 3 patches from this series to the

master branch now.

For the changes regarding propagated-inputs though, I'm not sure this it

the right direction. Firstly, I think it's possible (and maybe

desirable) to keep the 'fix-paths phase, even if the inputs are changed

to be propagated.

I know you say this is related to yosys show in the commit message, can

you elaborate on why these packages are necessary to have in the users

environment?

Thanks,

Chris

Christopher Baines <mail@cbaines.net> writes:

Nice, thank you.

Yosys' "show" command produces its output by building and executing a

shell command that invokes "dot" or "xdot" on the user's data. The

implementation is in passes/cmds/show.cc[0]; the code for invoking dot

for instance looks like

#define DOT_CMD "dot -T%s '%s' > '%s.new' && mv '%s.new' '%s'"

(...)

std::string cmd = stringf(DOT_CMD, format.c_str(), dot_file.c_str(), out_file.c_str(), out_file.c_str(), out_file.c_str());

log("Exec: %s\n", cmd.c_str());

if (run_command(cmd) != 0)

log_cmd_error("Shell command failed!\n");

Obviously this works only if "dot" is in the user's PATH (as Yosys

blindly assumes), so the graphviz package must be installed as well or

the "show" command will be broken. Similarly for the "fuser" and "xdot"

executables, which by default are invoked to provide graphical output

(though their use can be overridden by a setting at runtime).

I find this business of generating shell commands a bit ugly but at

least it creates only a loose coupling between the executables: The

shell is free to select a suitable "dot" to run, so the user can

substitute their own as easily as changing their PATH.

In constrast, the existing 'fix-paths phase tightens this coupling

considerably, as it embeds in the DOT_CMD string above the full path to

a specific "dot" executable in the store. This ties the two executables

together completely, making it very difficult to change which "dot" is

used by Yosys without rebuilding the package.

I see no advantage to coupling the executables together tightly this

way, and in fact it seems counter to what is implied by the code above.

(Note also the "ABCEXTERNAL" build variable adjusted in a previous patch

is provided specifically to allow users to swap in their own version of

abc, which a different Yosys command invokes.) I'd rather we propagate

the packages needed to ensure Yosys works as expected out-of-the-box,

then leave the user free to override its behaviour as they see fit.

--

Simon South

simon@simonsouth.net

[0] https://github.com/YosysHQ/yosys/blob/1979e0b1f2482dbf0562f5116ab444280a377773/passes/cmds/show.cc

Am Mittwoch, dem 08.02.2023 um 19:35 -0500 schrieb Simon South:

When propagating inputs, it creates the same tight coupling as

hardcoding would, but with worse UX (think propagating conflicts).

You should instead try:

std::optional<std::string> dot_bin = which("dot")

if (!dot_bin)

dot_bin = "@dot@";

std::string cmd = string(DOT_CMD, *dot_bin, ...)

Cheers

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

That would be a nice approach but I don't really want to start modifying

the source code.

I'll send out a v3 of the remaining patches that keeps the 'fix-paths

phase (and updates Yosys to 0.26, since that came out yesterday).

Embedding store references directly in source code still feels ugly to

me but I can see how propagating packages just creates a different

problem. Pity there isn't an elegant way to specify a package should

"use this input, but not exclusively" (though at least there's "guix

build --with-input").

--

Simon South

simon@simonsouth.net

Here's a third revision of the remaining two patches in this series. These

updated patches now

- Remove the remaining propagated inputs (abc and z3) from the yosys package

via changes to its "fix-paths" and "use-external-abc" phases, and

- Update Yosys to version 0.26.

Note the second patch adds python and python-click as new propagated inputs.

The Click library is needed by the "yosys-witness" command, and including both

it and the Python interpreter in propagated-inputs is the only combination

I've found that will cause Guix to set GUIX_PYTHONPATH appropriately on

installation so this command can function.

This appears to be supported by the Guix manual (in the sections "Search

Paths" and "Python Modules") but if there's a better approach, please let me

know.

As usual, I've tested these changes on x86-64 and AArch64 and all appears

well.

--

Simon South

simon@simonsouth.net

Simon South (2):

gnu: yosys: Do not propagate any inputs.

gnu: yosys: Update to 0.26.

gnu/packages/fpga.scm | 56 +++++++++++++++++++------------------------

1 file changed, 24 insertions(+), 32 deletions(-)

base-commit: b8f6ead5faac3c1b9a8fa6e060c00cf0917e884e

--

2.39.1

"fix-paths" phase; in "use-external-abc" phase, use complete path to "abc"

executable in store.

[propagated-inputs]: Remove, moving abc and z3 from here...

[inputs]: ...to here.

---

gnu/packages/fpga.scm | 18 +++++++++++-------

1 file changed, 11 insertions(+), 7 deletions(-)

[source]: Disable unnecessary recursive checkout.

[arguments]<#:phases>: Expand "fix-paths" phase to match new version; remove

obsolete "fix-iverilog-references" phase.

[inputs]: Add gtkwave, zlib.

[propagated-inputs]: Add python, python-click.

---

gnu/packages/fpga.scm | 42 +++++++++++++++---------------------------

1 file changed, 15 insertions(+), 27 deletions(-)

Simon South <simon@simonsouth.net> writes:

Thanks for the update, I made a tweak and pushed these to master as

8553148dfb91a9957b95c7bc6bc108cc0a973f9e.

I believe the good approach to do this is to wrap the relevant

script. I've gone ahead and changed the patch to do this rather than add

the propagated inputs. Do let me know if there's a problem with this

approach though, as I don't know how to test this software as well as

you.

Thanks again,

Chris