Re: [Libguestfs] [nbdkit] Two POSIX questions for you ...
On Mon, Oct 09, 2023 at 01:32:07PM +0200, Laszlo Ersek wrote:
> >> 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
> >> 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.
>
> The runtime dependency is already there in our logscript interface; the
>
> name=(a b c ... z)
>
> syntax is already bash-only, for defining a shell array.
The man page for nbdkit-log-filter does not mention extents=()
anywhere, and exports=("") is only shown as a single sample log
output, without mention in the LOG SCRIPT section. I checked both
1.34.4 I have pre-installed on Fedora 38, and
filters/log/nbdkit-log-filter.pod at the top of development. Merely
stating "Strings and lists are shell-quoted." or that "Other
parameters like C are turned into shell variables C<$offset>
etc." does not properly describe which items passed to the logscript
will actually be lists under which exported names, whether or not we
choose to stick to bash-only syntax. So we already have a bug that
our documentation is incomplete; and therefore, if we change what we
output, we can also use that as a chance to rectify our documentation
bug to what we really want to support.
>
> So the question is basically how to best emulate an array in the POSIX
> shell. Some (rough) options that occur to me:
>
> - Use named variables such as name_0, name_1, name_2, ... and so on.
> Requires eval tricks, and if the array is large, it creates many
> variables, which some shells (?) may have issues with.
If we're going to run out of memory from having too many strings to
pass through all extents information through the environ, we will do
so whether we do it as a single array variable or as multiple
individual variables.
>
> - Generate a shell function with a huge case statement; like "get_name
> 0" should print "a", "get_name 1" should print "b", etc. The caller
> would then do
>
> element=$(get_name $idx)
We'd also need to define something like $extents_max=N so that the
script can get correct bounds for the loop it writes around the
$(get_name $idx) calls.
>
> - write the elements of the array to a text file (one, quoted, element
> per line), and then use a combination of "tail" and "head" for fetching
> the right line. Incredibly slow, of course.
>
> ... I'm sure stackoverflow has further / better ideas for emulating
> arrays in the POSIX shell.
>
> And then, because keeping the current (fast, but nonportable) solution
> would be nice, we should probably add a new argument for the log filter,
> "compat" or "posix" or some such, which would select the more restricted
> interface.
Yes, having a way to fine-tune what we output for the scripts
consumption would be nice, and perhaps extensible (JSON output,
anyone?).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [nbdkit] Two POSIX questions for you ...
On Mon, Oct 09, 2023 at 12:12:02PM +0100, Richard W.M. Jones wrote:
> 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.
Correct.
>
> [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.]
It's not just Debian's dash, but alpine's choice of busybox (a variant
of ash with distinct differences from dash) that can also bite you
when using non-POSIX constructs. Debian pioneered a shellcheck app
(also available in the ShellCheck package in Fedora) to try and detect
scripts that depend on bashisms; but I'm not sure it would help the
case of where you are embedding constructs that will be run down the
road under system(3) in a much larger file that is not constrained by
a limited shell.
>
> 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.
Anywhere you call system(3), you can pass "/path/to/known/shell -c '"
+ shell_quote(original script) + "'" as the arguments to system to
control which shell syntax you support in the original script. But I
agree that it is not always trivial.
>
> 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.]
Yeah, it's not trivial to decide how to support --filter=log
logscript=... under Windows - we could require the user to have
something like Cygwin or mingw installed so that there is a 'sh.exe'
somewhere, but that's not reliable; and native Windows scripting is so
different from anything POSIX that it's probably easier to just
declare logscript= unsupported on that platform (if the rest of the
log filter can still be used).
>
> > 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.
Yep, that's a definite bashism in the log filter. Several ideas:
maybe we change to call out to bash (directly via exec*(), or
indirectly via the system("/path/to/bash -c 'original script'")
trick). Maybe we come up with a way for the command-line client of
the log filter to specify which syntax they expect to be handed to
their log script (if they don't specify a syntax, they get output that
is parseable only by bash). I'm open to other ideas.
> >
> > In the bug tracker, comment
> > 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.
Only for that one filter, not for nbdkit in general. But yeah, it's
still a rather strong dependency, and if we could avoid it, that would
be nicer.
>
> > > Secondly while looking into this I was trying variations on:
> > >
> > > $ POSIXLY_CORRECT=1 ./nbdkit -fv data '1 2 3' --run 'nbdinfo $uri'
> > >
>
Re: [Libguestfs] [nbdkit] Two POSIX questions for you ...
On Tue, Oct 10, 2023 at 09:05:43AM +0200, Laszlo Ersek wrote: > On 10/9/23 16:51, Richard W.M. Jones wrote: > > > > So one thing we could do for this test is to require (for the test) > > that /bin/sh is bash, something like the patch below. I was only able > > to test this in the positive case. > > > > diff --git a/tests/test-log-script-info.sh b/tests/test-log-script-info.sh > > index fa9b2ed32..d65f6415d 100755 > > --- a/tests/test-log-script-info.sh > > +++ b/tests/test-log-script-info.sh > > @@ -38,6 +38,10 @@ requires_run > > requires_nbdinfo > > requires_filter log > > > > +# This test implicitly assumes /bin/sh is bash, see: > > +# https://listman.redhat.com/archives/libguestfs/2023-October/032767.html > > +BASH_VERSION=no requires /bin/sh -c 'test "x$BASH_VERSION" != "xno"' > > + > > if ! nbdinfo --help | grep -- --map ; then > > echo "$0: nbdinfo --map option required to run this test" > > exit 77 > > > > > > Clever! > > Reviewed-by: Laszlo Ersek > > You can test it with "dash" BTW (it must be available in Fedora, because > it is available in EPEL-9): > > BASH_VERSION=no /bin/bash -c 'test "x$BASH_VERSION" != "xno"; echo $?' > BASH_VERSION=no /bin/dash -c 'test "x$BASH_VERSION" != "xno"; echo $?' Thanks - after testing with dash as you suggested, I pushed this change which is similar in spirit: https://gitlab.com/nbdkit/nbdkit/-/commit/99712c09835ae430912aed4848579fdfab01a576 Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [nbdkit] Two POSIX questions for you ...
On 10/9/23 16:51, Richard W.M. Jones wrote: > > So one thing we could do for this test is to require (for the test) > that /bin/sh is bash, something like the patch below. I was only able > to test this in the positive case. > > diff --git a/tests/test-log-script-info.sh b/tests/test-log-script-info.sh > index fa9b2ed32..d65f6415d 100755 > --- a/tests/test-log-script-info.sh > +++ b/tests/test-log-script-info.sh > @@ -38,6 +38,10 @@ requires_run > requires_nbdinfo > requires_filter log > > +# This test implicitly assumes /bin/sh is bash, see: > +# https://listman.redhat.com/archives/libguestfs/2023-October/032767.html > +BASH_VERSION=no requires /bin/sh -c 'test "x$BASH_VERSION" != "xno"' > + > if ! nbdinfo --help | grep -- --map ; then > echo "$0: nbdinfo --map option required to run this test" > exit 77 > > Clever! Reviewed-by: Laszlo Ersek You can test it with "dash" BTW (it must be available in Fedora, because it is available in EPEL-9): BASH_VERSION=no /bin/bash -c 'test "x$BASH_VERSION" != "xno"; echo $?' BASH_VERSION=no /bin/dash -c 'test "x$BASH_VERSION" != "xno"; echo $?' Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [nbdkit] Two POSIX questions for you ...
So one thing we could do for this test is to require (for the test) that /bin/sh is bash, something like the patch below. I was only able to test this in the positive case. diff --git a/tests/test-log-script-info.sh b/tests/test-log-script-info.sh index fa9b2ed32..d65f6415d 100755 --- a/tests/test-log-script-info.sh +++ b/tests/test-log-script-info.sh @@ -38,6 +38,10 @@ requires_run requires_nbdinfo requires_filter log +# This test implicitly assumes /bin/sh is bash, see: +# https://listman.redhat.com/archives/libguestfs/2023-October/032767.html +BASH_VERSION=no requires /bin/sh -c 'test "x$BASH_VERSION" != "xno"' + if ! nbdinfo --help | grep -- --map ; then echo "$0: nbdinfo --map option required to run this test" exit 77 -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [nbdkit] Two POSIX questions for you ...
On Mon, Oct 09, 2023 at 01:32:07PM +0200, Laszlo Ersek wrote:
> On 10/9/23 13:12, Richard W.M. Jones wrote:
> > 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
> >> 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.
>
> The runtime dependency is already there in our logscript interface; the
>
> name=(a b c ... z)
>
> syntax is already bash-only, for defining a shell array.
Hmm, the extents are indeed passed as a flattened array, ie.
extents=( "" [repeats ...])
I don't think I was aware this wasn't in POSIX.
However that's a dependency from _nbdkit-log-filter_ to bash, which is
slightly different. (Nevertheless it'd be nice if we could remove it ...)
> So the question is basically how to best emulate an array in the POSIX
> shell. Some (rough) options that occur to me:
>
> - Use named variables such as name_0, name_1, name_2, ... and so on.
> Requires eval tricks, and if the array is large, it creates many
> variables, which some shells (?) may have issues with.
>
> - Generate a shell function with a huge case statement; like "get_name
> 0" should print "a", "get_name 1" should print "b", etc. The caller
> would then do
>
> element=$(get_name $idx)
>
> - write the elements of the array to a text file (one, quoted, element
> per line), and then use a combination of "tail" and "head" for fetching
> the right line. Incredibly slow, of course.
>
> ... I'm sure stackoverflow has further / better ideas for emulating
> arrays in the POSIX shell.
>
> And then, because keeping the current (fast, but nonportable) solution
> would be nice, we should probably add a new argument for the log filter,
> "compat" or "posix" or some such, which would select the more restricted
> interface.
Yup.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
Re: [Libguestfs] [nbdkit] Two POSIX questions for you ...
On 10/9/23 13:12, Richard W.M. Jones wrote:
> 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
>> 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.
The runtime dependency is already there in our logscript interface; the
name=(a b c ... z)
syntax is already bash-only, for defining a shell array.
So the question is basically how to best emulate an array in the POSIX
shell. Some (rough) options that occur to me:
- Use named variables such as name_0, name_1, name_2, ... and so on.
Requires eval tricks, and if the array is large, it creates many
variables, which some shells (?) may have issues with.
- Generate a shell function with a huge case statement; like "get_name
0" should print "a", "get_name 1" should print "b", etc. The caller
would then do
element=$(get_name $idx)
- write the elements of the array to a text file (one, quoted, element
per line), and then use a combination of "tail" and "head" for fetching
the right line. Incredibly slow, of course.
... I'm sure stackoverflow has further / better ideas for emulating
arrays in the POSIX shell.
And then, because keeping the current (fast, but nonportable) solution
would be nice, we should probably add a new argument for the log filter,
"compat" or "posix" or some such, which would select the more restricted
interface.
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [nbdkit] Two POSIX questions for you ...
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
> 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], )) {
> 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
Re: [Libguestfs] [nbdkit] Two POSIX questions for you ...
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. 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
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().
>
> - - -
>
> 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], )) {
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.
Laszlo
>
> Rich.
>
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [nbdkit] Two POSIX questions for you ...
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? - - - 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 (?) 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
