all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Simon South <simon@simonsouth.net>
To: Christopher Baines <mail@cbaines.net>
Cc: 60429@debbugs.gnu.org
Subject: [bug#60429] [PATCH v2 4/5] gnu: yosys: Propagate external dependencies.
Date: Wed, 08 Feb 2023 19:35:30 -0500	[thread overview]
Message-ID: <87v8kb7k19.fsf@simonsouth.net> (raw)
In-Reply-To: <87357g6pfz.fsf@cbaines.net> (Christopher Baines's message of "Wed, 08 Feb 2023 17:14:59 +0000")

Christopher Baines <mail@cbaines.net> 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
simon@simonsouth.net

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




  reply	other threads:[~2023-02-09  0:36 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-30 15:58 [bug#60429] [PATCH 0/5] gnu: yosys: Update to 0.24 Simon South
2022-12-30 16:00 ` [bug#60429] [PATCH 1/5] gnu: yosys: Update source and home-page URLs Simon South
2022-12-30 16:00 ` [bug#60429] [PATCH 2/5] gnu: yosys: Use new package style Simon South
2022-12-30 16:00 ` [bug#60429] [PATCH 3/5] gnu: yosys: Use external abc Simon South
2022-12-30 16:00 ` [bug#60429] [PATCH 4/5] gnu: yosys: Propagate external dependencies Simon South
2022-12-30 16:00 ` [bug#60429] [PATCH 5/5] gnu: yosys: Update to 0.24 Simon South
2023-01-08 18:31 ` [bug#60429] [PATCH v2 0/5] " Simon South
2023-01-08 18:31   ` [bug#60429] [PATCH v2 1/5] gnu: yosys: Update source and home-page URLs Simon South
2023-01-08 18:31   ` [bug#60429] [PATCH v2 2/5] gnu: yosys: Use new package style Simon South
2023-01-08 18:31   ` [bug#60429] [PATCH v2 3/5] gnu: yosys: Use external abc Simon South
2023-01-08 18:31   ` [bug#60429] [PATCH v2 4/5] gnu: yosys: Propagate external dependencies Simon South
2023-02-08 17:14     ` Christopher Baines
2023-02-09  0:35       ` Simon South [this message]
2023-02-09  5:29         ` Liliana Marie Prikler
2023-02-09 16:47           ` Simon South
2023-01-08 18:31   ` [bug#60429] [PATCH v2 5/5] gnu: yosys: Update to 0.25 Simon South
2023-02-10 13:16 ` [bug#60429] [PATCH v3 0/5] gnu: yosys: Update to 0.24 Simon South
2023-02-10 13:16   ` [bug#60429] [PATCH v3 4/5] gnu: yosys: Do not propagate any inputs Simon South
2023-02-10 13:16   ` [bug#60429] [PATCH v3 5/5] gnu: yosys: Update to 0.26 Simon South
2023-02-11 20:38   ` bug#60429: [PATCH v3 0/5] gnu: yosys: Update to 0.24 Christopher Baines

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87v8kb7k19.fsf@simonsouth.net \
    --to=simon@simonsouth.net \
    --cc=60429@debbugs.gnu.org \
    --cc=mail@cbaines.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.