From: "Ludovic Courtès" <ludo@gnu.org>
To: Rutger van Beusekom <rutger.van.beusekom@verum.com>
Cc: Guile Devel <guile-devel@gnu.org>
Subject: Re: guile pipeline do-over
Date: Sat, 07 Mar 2020 16:46:47 +0100 [thread overview]
Message-ID: <87imjgi2vs.fsf@gnu.org> (raw)
In-Reply-To: <8736al24jt.fsf@verum.com> (Rutger van Beusekom's message of "Fri, 06 Mar 2020 10:52:54 +0100")
Hi Rutger!
Rutger van Beusekom <rutger.van.beusekom@verum.com> skribis:
> This patch replaces open-process with piped-process in posix.c and
> reimplement open-process with piped-process in popen.scm. This allows
> setting up a pipeline in guile scheme using the new pipeline procedure
> in popen.scm and enables its use on operating systems which happen to
> lack the capability to fork, but do offer the capability captured by
> start_child (see posix.c).
Nice! That’s definitely very useful to have. We’ll need to check what
Andy thinks, but I think it can be added in the 3.0 series.
> From 3c8f5d534419418234cfe7d3eda8227951bc208a Mon Sep 17 00:00:00 2001
> From: Rutger van Beusekom <rutger.van.beusekom@verum.com>
> Date: Mon, 2 Mar 2020 10:38:57 +0100
> Subject: [PATCH] Allow client code to create pipe pairs when opening a
> process.
>
> * libguile/posix.c (scm_piped_process): Replace open_process by piped_process.
Could you mention functions renamed/removed here? The ChangeLog format
is about boringly listing all the language-entity-level changes. :-)
> * module/ice-9/popen.scm (pipe->fdes): Convert pipe pair to fdes pair.
> (open-process): Implement open-process with piped-process.
> (pipeline): Implement a pipeline with piped-process.
> static SCM
> -scm_open_process (SCM mode, SCM prog, SCM args)
> -#define FUNC_NAME "open-process"
> +scm_piped_process (SCM prog, SCM args, SCM from, SCM to)
> +#define FUNC_NAME "piped-process"
> +/* SCM_DEFINE (scm_piped_process, "piped-process", 2, 2, 0, */
> +/* (SCM prog, SCM args, SCM from, SCM to), */
> +/* "Execute the command indicated by @var{prog} with arguments @var(args),\n" */
> +/* "optionally connected by an input and an output pipe.\n" */
> +/* "@var(from) and @var(to) are either #f or a valid file descriptor\n" */
> +/* "of an input and an output pipe, respectively.\n" */
> +/* "\n" */
> +/* "This function returns the PID of the process executing @var(prog)." */
> +/* "\n" */
> +/* "Example:\n" */
> +/* "(let ((p (pipe)))\n" */
> +/* " (piped-process \"echo\" '(\"foo\" \"bar\")\n" */
> +/* " (cons (port->fdes (car p))\n" */
> +/* " (port->fdes (cdr p))))\n" */
> +/* " (display (read-string (car p))))\n" */
> +/* "(let ((p (pipe)))\n" */
> +/* " (read-string (piped-process \"echo\" '(\"foo\" \"bar\")\n" */
> +/* " (port->fdes (car p)))))\n") */
> +/* #define FUNC_NAME scm_piped_process */
I guess you can remove the commented-out bits…
> - int c2p[2]; /* Child to parent. */
> - int p2c[2]; /* Parent to child. */
> + int c2p[2] = {}; /* Child to parent. */
> + int p2c[2] = {}; /* Parent to child. */
… and this hunk, to minimize change.
> +++ b/module/ice-9/popen.scm
> @@ -22,9 +22,10 @@
> #:use-module (rnrs bytevectors)
> #:use-module (ice-9 binary-ports)
> #:use-module (ice-9 threads)
> + #:use-module (srfi srfi-1)
> #:use-module (srfi srfi-9)
> #:export (port/pid-table open-pipe* open-pipe close-pipe open-input-pipe
> - open-output-pipe open-input-output-pipe))
> + open-output-pipe open-input-output-pipe pipe->fdes piped-process pipeline))
I would not export ‘pipe->fdes’. I’m not sure about exporting
‘piped-process’: it’s a bit low-level and we might want to reserve
ourselves the possibility to change it, like this patch does actually.
WDYT?
> +(define (open-process mode command . args)
> + (let* ((from (and (or (equal? mode OPEN_READ) (equal? mode OPEN_BOTH)) (pipe->fdes)))
> + (to (and (or (equal? mode OPEN_WRITE) (equal? mode OPEN_BOTH)) (pipe->fdes)))
> + (pid (piped-process command args from to)))
> + (values (and from (fdes->inport (car from))) (and to (fdes->outport (cdr to))) pid)))
Please wrap lines to 80 chars.
I suggest using ‘string=?’ above instead of ‘equal?’. Also, could you
add a docstring?
> +(define (pipeline procs)
> + "Execute a pipeline of @code(procs) -- where a proc is a list of a
> +command and its arguments as strings -- returning an input port to the
> +end of the pipeline, an output port to the beginning of the pipeline and
> +a list of PIDs of the @code(procs)"
Perhaps s/procs/commands/ would be clearer? Also, @var{commands}
instead of @code.
Could you also add an entry in doc/ref/*.texi, in the “Pipes” node,
perhaps with one of the examples you gave?
> +++ b/test-suite/tests/popen.test
> @@ -211,3 +211,37 @@ exec 2>~a; read REPLY"
> (let ((st (close-pipe (open-output-pipe "exit 1"))))
> (and (status:exit-val st)
> (= 1 (status:exit-val st)))))))
> +
> +
> +;;
> +;; pipeline related tests
> +;;
> +
> +(use-modules (ice-9 receive))
> +(use-modules (ice-9 rdelim))
Please move these to the top-level ‘define-module’ form.
One last thing: we’d need you to assign copyright to the FSF for this.
We can discuss it off-line if you want.
Thank you for this great and long overdue addition!
Ludo’.
next prev parent reply other threads:[~2020-03-07 15:46 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-06 9:52 guile pipeline do-over Rutger van Beusekom
2020-03-07 15:46 ` Ludovic Courtès [this message]
2020-03-10 7:35 ` Rutger van Beusekom
2020-03-10 8:54 ` Linus Björnstam
2020-03-10 10:37 ` Rutger van Beusekom
2020-03-26 9:09 ` Ludovic Courtès
2020-04-04 8:01 ` Rutger van Beusekom
2020-05-16 20:38 ` Ludovic Courtès
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
List information: https://www.gnu.org/software/guile/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87imjgi2vs.fsf@gnu.org \
--to=ludo@gnu.org \
--cc=guile-devel@gnu.org \
--cc=rutger.van.beusekom@verum.com \
/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.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).