Subject: [bug#60429] [PATCH v2 4/5] gnu: yosys: Propagate external dependencies.

From: Simon South <[email-masked]>
Date: Wed, 08 Feb 2023 19:35:30 -0500

Christopher Baines <[email-masked]> writes:
> Thanks Simon, I've pushed the first 3 patches from this series to the
> master branch now.

Nice, thank you.

> 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?

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
[email-masked]

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