On Mon, Oct 09, 2023 at 12:57:14PM +0200, Laszlo Ersek wrote:
> On 10/9/23 09:46, Richard W.M. Jones wrote:
> > Hi Eric, a couple of POSIX questions for you from nbdkit.
> >
> > The first question is from an AUR comment on nbdkit:
> >
> > https://aur.archlinux.org/packages/nbdkit#comment-937381
> >
> > I think there's a bash-ism in the logscript parameter in this test:
> >
> >
> > https://gitlab.com/nbdkit/nbdkit/-/blame/master/tests/test-log-script-info.sh#L51
> >
> > I believe it is happening in the $(( .. )) expression. How do we
> > write that so it'll work in a posix shell?
>
> (I'm not Eric, but curious! :) )
>
> Here I think we should just explicitly insist on bash.
Unfortunately we can't do that, not easily anyway.
We use bash for writing the test scripts because it's the sane choice
for shell programming, and we declare /bin/bash (or something with
'env') at the top of each of those scripts. This introduces a *build*
dependency from nbdkit to /bin/bash. That's all fine.
However when nbdkit is installed in production & runs an external
script, it uses system(3) which uses /bin/sh. That might not be bash,
and indeed bash might not even be installed on the same machine as
nbdkit.
[Note: I think the whole Debian dash-for-/bin/sh is the stupidest idea
I ever heard, but (a) it's a thing and (b) there's BSD and Macs where
they don't want to use bash for licensing reasons.]
Arguably for the tests we could have some way to cause nbdkit to use
/bin/bash instead of /bin/sh when running external scripts, but it's
major complexity to implement.
So we gotta use a lowest common denominator for these external
scripts, even in our tests. Note only this one test is affected.
[Aside: Windows is a whole separate kettle of fish. Currently the
Windows port of nbdkit doesn't support --run for other reasons, but if
it did it would probably run some Windows-ish thing (COMMAND.COM?
Power Shell?). I'm not sure how Windows behaves for the other
external commands we try to run like logscript.]
> Shell arrays are
> a bash-specific feature, and the extent array is deeply ingrained. See
> especially commit df63b23b6280 ("log: Use strict shell quoting for every
> parameter displayed in the log file.", 2021-01-04). In particular the
> assignment
>
> extents=(0x0 0x8000 "hole,zero" 0x8000 0x8000 "")
>
> turns "extents" into an array variable.
>
> In the bug tracker, comment
> <https://aur.archlinux.org/packages/nbdkit#comment-937375> says, "Thus
> this is an upstream issue; their scripts are calling sh when they should
> be calling bash". I think that's correct; for logscript=..., we should
> require /bin/bash in the manual, and execute the script with /bin/bash
> explicitly, not just system().
This would create a runtime dependency from nbdkit to /bin/bash which
I'd like to avoid.
> > Secondly while looking into this I was trying variations on:
> >
> > $ POSIXLY_CORRECT=1 ./nbdkit -fv data '1 2 3' --run 'nbdinfo $uri'
> >
> > This doesn't actually cause bash to emulate a posix shell, but it does
> > uncover a different bug:
> >
> > nbdkit: error: raw|base64|data parameter must be specified exactly once
> >
> > This seems to be happening because getopt_long in wrapper.c behaves
> > somehow differently parsing when POSIXLY_CORRECT is set. However I
> > couldn't work out exactly why.
> >
> >
> > https://gitlab.com/nbdkit/nbdkit/-/blob/master/wrapper.c?ref_type=heads#L278
> >
> > I guess the wrapper ought to work if POSIXLY_CORRECT is set (?)
>
> IIRC, POSIXLY_CORRECT makes getopt() stop parsing options when the first
> non-option argument (i.e., first operand) is reached. So "-f" and "-v"
> are taken as options, then the two arguments "data" and '1 2 3' are
> taken as operands, and then "--run" and the rest are taken as operands
> as well.
>
> I think the following loop is relevant:
>
> /* Are there any non-option arguments? */
> if (optind < argc) {
> /* Ensure any further parameters can never be parsed as options by
> * real nbdkit.
> */
> passthru ("--");
>
> /* The first non-option argument is the plugin name. If it is a
> * short name then rewrite it.
> */
> if (is_short_name (argv[optind])) {
> const char *language;
>
> /* Plugins written in scripting languages. */
> if (is_script_plugin (argv[optind], &language)) {
> passthru_format ("%s/plugins/%s/.libs/nbdkit-%s-plugin." SOEXT,
> builddir, language, language);
> passthru_format ("%s/plugins/%s/nbdkit-%s-plugin",
> builddir, argv[optind], argv[optind]);
> }
> /* Otherwise normal plugins written in C or other languages that
> * compile to .so files.
> */
> else {
> passthru_format ("%s/plugins/%s/.libs/nbdkit-%s-plugin." SOEXT,
> builddir, argv[optind], argv[optind]);
> }
> ++optind;
> }
>
> /* Everything else is passed through without rewriting. */
> while (optind < argc) {
> passthru (argv[optind]);
> ++optind;
> }
> }
>
> With POSIXLY_CORRECT set, the "Everything else is passed through without
> rewriting" logic extends to "--run" etc. I'm not sure how that would
> lead to the specific error message, though.
That's annoying :-(
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
